[FFmpeg-devel] [PATCH] Get rid of p
Michael Niedermayer
michaelni
Sun Jun 8 22:09:52 CEST 2008
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:
> >> Hello,
> >>
> >> 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
> >> ctx;
> >> - '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)
>
> > 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?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080608/b7679327/attachment.pgp>
More information about the ffmpeg-devel
mailing list