# [CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly 4 messages from 2009-01-02 to 2009-01-02. Participants: Johannes Schindelin, Boyd Stephen Smith Jr.. Thread: https://gitlist.dev/t/16968 ## Johannes Schindelin, 2009-01-02 18:07 Subject: [CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0901021240270.27818%40racer ``` It is not a good practice to prefer performance over readability in something as performance uncritical as finding the trailing slash of argv[0]. So avoid head-scratching by making the loop user-readable, and not hyper-performance-optimized. Signed-off-by: Johannes Schindelin --- git.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/git.c b/git.c index 940a498..e0d9071 100644 --- a/git.c +++ b/git.c @@ -428,9 +428,8 @@ int main(int argc, const char **argv) * name, and the dirname as the default exec_path * if we don't have anything better. */ - do - --slash; - while (cmd <= slash && !is_dir_sep(*slash)); + while (cmd <= slash && !is_dir_sep(*slash)) + slash--; if (cmd <= slash) { *slash++ = 0; git_set_argv0_path(cmd); -- 1.6.1.rc3.224.g95ac9 ``` ## Boyd Stephen Smith Jr., 2009-01-02 18:28 Subject: Re: [CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly Message-ID: <200901021228.07599.bss@iguanasuicide.net> URL: https://gitlist.dev/e/200901021228.07599.bss%40iguanasuicide.net In-Reply-To: ``` On Friday 2009 January 02 12:07:52 you wrote: > It is not a good practice to prefer performance over readability in > something as performance uncritical as finding the trailing slash > of argv[0]. > > So avoid head-scratching by making the loop user-readable, and not > hyper-performance-optimized. > - do > - --slash; > - while (cmd <= slash && !is_dir_sep(*slash)); > + while (cmd <= slash && !is_dir_sep(*slash)) > + slash--; What confused people? The predecrement or the do/while? Should people that don't understand one of those be hacking on git? That said, I'm not opposed to the patch. It is easier on the eyes, though I prefer the one-liner: for (; cmd <= slash && !is_dir_sep(*slash); --slash); -- Boyd Stephen Smith Jr. ,= ,-_-. =. bss@iguanasuicide.net ((_/)o o(\_)) ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-' http://iguanasuicide.net/ \_/ ``` ## Johannes Schindelin, 2009-01-02 18:49 Subject: Re: [CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0901021947170.30769%40pacific.mpi-cbg.de In-Reply-To: <200901021228.07599.bss@iguanasuicide.net> ``` Hi, On Fri, 2 Jan 2009, Boyd Stephen Smith Jr. wrote: > On Friday 2009 January 02 12:07:52 you wrote: > > It is not a good practice to prefer performance over readability in > > something as performance uncritical as finding the trailing slash > > of argv[0]. > > > > So avoid head-scratching by making the loop user-readable, and not > > hyper-performance-optimized. > > > - do > > - --slash; > > - while (cmd <= slash && !is_dir_sep(*slash)); > > + while (cmd <= slash && !is_dir_sep(*slash)) > > + slash--; > > What confused people? The predecrement or the do/while? Should people that > don't understand one of those be hacking on git? > > That said, I'm not opposed to the patch. It is easier on the eyes, though I > prefer the one-liner: > for (; cmd <= slash && !is_dir_sep(*slash); --slash); As I mentioned in the commit message: readability is something to be cherished and worshipped. For your pleasure, I will not go into details about the motions my bowels went through when I looked at those three lines. Or your single line, for that matter. Ciao, Dscho ``` ## Boyd Stephen Smith Jr., 2009-01-02 20:04 Subject: Re: [CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly Message-ID: <200901021404.05811.bss@iguanasuicide.net> URL: https://gitlist.dev/e/200901021404.05811.bss%40iguanasuicide.net In-Reply-To: ``` On Friday 2009 January 02 12:49:53 Johannes Schindelin wrote: > > > - do > > > - --slash; > > > - while (cmd <= slash && !is_dir_sep(*slash)); > > > + while (cmd <= slash && !is_dir_sep(*slash)) > > > + slash--; > > > > I prefer the one-liner: > > for (; cmd <= slash && !is_dir_sep(*slash); --slash); > > As I mentioned in the commit message: readability is something to be > cherished and worshipped. It's also subjective. I think my one-line is more readable than your two lines which is only slightly more readable than the original 3 lines. Or is there some objective readability metric that of which I'm just not aware? I also think that the lack of braces around your body on a separate line makes it harder to read and easier to break, but I understand that is the git coding style. > For your pleasure, I will not go into details about the motions my bowels > went through when I looked at those three lines. Or your single line, for > that matter. Please do, although privately if you like. I really don't see the problem the patch is trying to fix. -- Boyd Stephen Smith Jr. ,= ,-_-. =. bss@iguanasuicide.net ((_/)o o(\_)) ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-' http://iguanasuicide.net/ \_/ ```