[aur-dev] [PATCH] Add a logging function and logging fixins through the code

[Dan McGee]

On Wed, Jul 1, 2009 at 12:42 PM, Sebastian Nowicki<sebnow at gmail.com> wrote:
>
> On Jul 1, 2009, at 7:36 PM, Callan Barrett wrote:
>
>>
>> On 02/07/2009, at 1:31 AM, Sebastian Nowicki wrote:
>>
>>>
>>> Might be easier to just put the "if(LOGGING) {}" into the logit()
>>> function itself, assuming this is the desired behavior for all instances of
>>> logging.
>>
>> I feel like an idiot for not putting it there in the first place but now
>> that I think about it... if logging was turned off we would be doing 2 hits
>> to the DB for almost every single action needlessly if the check was inside
>> the function. So maybe it's not worth the simplicity?
>
> Ah, you mean the calls to username_from_sid() and pkgname_from_id()? If they
> are always called, you could put that in the function as well, and only pass
> in the parameters ($_COOKIE['AURSID'], $pid), respectively, etc.). Otherwise
> you have a good point.
>
> The current interface for the logit function is better (more flexible) than
> with the above suggested changes though. I'd probably stick with the
> redundancy.

I'd move the two function calls into logit() as well, as it looks like
they are always called.

Why are we inserting username when userid should be the foreign key we
use to reference users? I guess if you aren't a fan of joins it makes
sense, but DB normalization does have benefits, especially when you
decided to use integers anyway to represent the different operations.

The same holds true for package IDs, why not just use the IDs directly
and not names? Getting the same end result table (with usernames and
pkgnames) would be only a matter of two simple joins, not hard for a
relational DB to do.

In addition, magic numbers are not so much fun and make it hard to
decipher the calls at first glance. Maybe some sort of associative
array could help here, declared near the logit() function?

$logit_actions = array(
'upload'=>1,
'update'=>2,
...
);

And then a call would end up looking like:
logit(logit_actions('upload'), $userID, $packageID);

-Dan