[FFmpeg-devel] [PATCH] Get rid of p
Sun Jun 8 22:42:54 CEST 2008
Michael Niedermayer wrote:
> On Sun, Jun 08, 2008 at 08:57:21PM +0100, Ramiro Polla wrote:
>> Michael Niedermayer wrote:
>>> On Fri, Jun 06, 2008 at 05:09:27PM +0100, Ramiro Polla wrote:
>>>> By the message subject you probably asked yourself "what 'p'?".
>>>> Exactly, I ask myself the same question when reading the source.
>>>> I find one-letter variable names hard to grep for and hard to understand.
>>>> Are patches to get rid of them or use more descriptive names welcome?
>>>> Particular cases that come to my mind are:
>>>> - 's', used throughout FFmpeg to mean context. I suggest to change them to
>>>> - 'p', used in video encoders for something that I still have no idea what
>>>> is doing there.
>>>> Ramiro Polla
>>>> Index: utils.c
>>>> --- utils.c (revision 13671)
>>>> +++ utils.c (working copy)
>>>> @@ -2779,7 +2779,6 @@
>>>> int64_t parse_date(const char *datestr, int duration)
>>>> - const char *p;
>>>> int64_t t;
>>>> struct tm dt;
>>>> int i;
>>>> @@ -2808,12 +2807,11 @@
>>>> memset(&dt, 0, sizeof(dt));
>>>> - p = datestr;
>>> int64_t parse_date(const char *p, int duration)
>>> and your patch is much smaller
>> Hmmm... That goes against the whole purpose of my patch =)
> The purpose being to annoy me? :)
> It just factorizes the change into the unneeded variable copying removial
> and the cosmetic renaming of datestr->p (which i probably wont accept)
The purpose: renaming of datestr->p (which you probably wont accept)
I'm too lazy to write a patch to remove p = datestr; atm =). Might do it
some other time if someone doesn't beat me to it...
>>> we can discuss the renaming then seperately ...
>>> iam not sure if its good or not for the functions in your patch ...
>> I know my argumentation skills are close to NULL, but I'll try anyways...
>> Once you know what the function does, having small variable names might
>> be better, but if you're reading it for the first time to try and find
>> out what it does, having single-letter or non-descriptive names makes it
>> harder to understand. Or if you're debugging and stumble upon those
>> variables, it takes longer to find out what they actually are.
>> I use FFmpeg's source code as a basis to learn lots of things. I prefer
>> learning from well-written C code than textual descriptions like
>> textbooks or tutorials, and I'm probably not the only one.
>> Another reason is that searching for any of occurrence of some variable
>> becomes hard, since its name is part of many other variable names and
>> mostly anything.
> get cscope
>> I found this specially painful when reading mpeg code for the mimic
>> encoder. Long functions with lots of s around...
> would it really have helped if these where lots of ctx instead? If so
> iam curious how?
Ctrl+F ctx F3 F3 F3...
Or / ctx n n n... if you prefer
[^a-z]s[^a-z] expects the search function of whatever editor to support
regexp. Maybe it's just because I grew out of Windows, but regexps are
much less intuitive for me, even if they are more powerful, specially
for simple searches.
More information about the ffmpeg-devel