{"thread":{"id":"16968","subject":"[CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly","startedAt":"2009-01-02T18:07:52Z","lastAt":"2009-01-02T20:04:02Z","messageCount":4,"participants":["Johannes Schindelin","Boyd Stephen Smith Jr."],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"99179","messageId":"alpine.DEB.1.00.0901021240270.27818@racer","threadId":"16968","inReplyTo":null,"subject":"[CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-02T18:07:52Z","receivedAt":"2009-01-02T18:07:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIt is not a good practice to prefer performance over readability in\nsomething as performance uncritical as finding the trailing slash\nof argv[0].\n\nSo avoid head-scratching by making the loop user-readable, and not\nhyper-performance-optimized.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n git.c |    5 ++---\n 1 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/git.c b/git.c\nindex 940a498..e0d9071 100644\n--- a/git.c\n+++ b/git.c\n@@ -428,9 +428,8 @@ int main(int argc, const char **argv)\n \t * name, and the dirname as the default exec_path\n \t * if we don't have anything better.\n \t */\n-\tdo\n-\t\t--slash;\n-\twhile (cmd <= slash && !is_dir_sep(*slash));\n+\twhile (cmd <= slash && !is_dir_sep(*slash))\n+\t\tslash--;\n \tif (cmd <= slash) {\n \t\t*slash++ = 0;\n \t\tgit_set_argv0_path(cmd);\n-- \n1.6.1.rc3.224.g95ac9\n"},{"id":"99184","messageId":"200901021228.07599.bss@iguanasuicide.net","threadId":"16968","inReplyTo":"alpine.DEB.1.00.0901021240270.27818@racer","subject":"Re: [CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly","fromName":"Boyd Stephen Smith Jr.","fromEmail":"bss@iguanasuicide.net","sentAt":"2009-01-02T18:28:03Z","receivedAt":"2009-01-02T18:28:03Z","isPatch":true,"sender":{"key":"bss@iguanasuicide.net","avatar":"https://gravatar.com/avatar/84b95eeff194b816c1568b1339e63e4b229825298664a9037b9f1ec713ead1e3?d=mp&s=160"},"body":"On Friday 2009 January 02 12:07:52 you wrote:\n> It is not a good practice to prefer performance over readability in\n> something as performance uncritical as finding the trailing slash\n> of argv[0].\n>\n> So avoid head-scratching by making the loop user-readable, and not\n> hyper-performance-optimized.\n\n> -\tdo\n> -\t\t--slash;\n> -\twhile (cmd <= slash && !is_dir_sep(*slash));\n> +\twhile (cmd <= slash && !is_dir_sep(*slash))\n> +\t\tslash--;\n\nWhat confused people?  The predecrement or the do/while?  Should people that \ndon't understand one of those be hacking on git?\n\nThat said, I'm not opposed to the patch.  It is easier on the eyes, though I \nprefer the one-liner:\nfor (; cmd <= slash && !is_dir_sep(*slash); --slash);\n-- \nBoyd Stephen Smith Jr.                     ,= ,-_-. =. \nbss@iguanasuicide.net                     ((_/)o o(\\_))\nICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' \nhttp://iguanasuicide.net/                      \\_/     \n"},{"id":"99186","messageId":"alpine.DEB.1.00.0901021947170.30769@pacific.mpi-cbg.de","threadId":"16968","inReplyTo":"200901021228.07599.bss@iguanasuicide.net","subject":"Re: [CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-02T18:49:53Z","receivedAt":"2009-01-02T18:49:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 2 Jan 2009, Boyd Stephen Smith Jr. wrote:\n\n> On Friday 2009 January 02 12:07:52 you wrote:\n> > It is not a good practice to prefer performance over readability in\n> > something as performance uncritical as finding the trailing slash\n> > of argv[0].\n> >\n> > So avoid head-scratching by making the loop user-readable, and not\n> > hyper-performance-optimized.\n> \n> > -\tdo\n> > -\t\t--slash;\n> > -\twhile (cmd <= slash && !is_dir_sep(*slash));\n> > +\twhile (cmd <= slash && !is_dir_sep(*slash))\n> > +\t\tslash--;\n> \n> What confused people?  The predecrement or the do/while?  Should people that \n> don't understand one of those be hacking on git?\n> \n> That said, I'm not opposed to the patch.  It is easier on the eyes, though I \n> prefer the one-liner:\n> for (; cmd <= slash && !is_dir_sep(*slash); --slash);\n\nAs I mentioned in the commit message: readability is something to be \ncherished and worshipped.\n\nFor your pleasure, I will not go into details about the motions my bowels \nwent through when I looked at those three lines.  Or your single line, for \nthat matter.\n\nCiao,\nDscho\n"},{"id":"99199","messageId":"200901021404.05811.bss@iguanasuicide.net","threadId":"16968","inReplyTo":"alpine.DEB.1.00.0901021947170.30769@pacific.mpi-cbg.de","subject":"Re: [CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly","fromName":"Boyd Stephen Smith Jr.","fromEmail":"bss@iguanasuicide.net","sentAt":"2009-01-02T20:04:02Z","receivedAt":"2009-01-02T20:04:02Z","isPatch":true,"sender":{"key":"bss@iguanasuicide.net","avatar":"https://gravatar.com/avatar/84b95eeff194b816c1568b1339e63e4b229825298664a9037b9f1ec713ead1e3?d=mp&s=160"},"body":"On Friday 2009 January 02 12:49:53 Johannes Schindelin wrote:\n> > > -\tdo\n> > > -\t\t--slash;\n> > > -\twhile (cmd <= slash && !is_dir_sep(*slash));\n> > > +\twhile (cmd <= slash && !is_dir_sep(*slash))\n> > > +\t\tslash--;\n> >\n> > I prefer the one-liner:\n> > for (; cmd <= slash && !is_dir_sep(*slash); --slash);\n>\n> As I mentioned in the commit message: readability is something to be\n> cherished and worshipped.\n\nIt's also subjective.  I think my one-line is more readable than your two \nlines which is only slightly more readable than the original 3 lines.  Or is \nthere some objective readability metric that of which I'm just not aware?\n\nI also think that the lack of braces around your body on a separate line makes \nit harder to read and easier to break, but I understand that is the git \ncoding style.\n\n> For your pleasure, I will not go into details about the motions my bowels\n> went through when I looked at those three lines.  Or your single line, for\n> that matter.\n\nPlease do, although privately if you like.  I really don't see the problem the \npatch is trying to fix.\n-- \nBoyd Stephen Smith Jr.                     ,= ,-_-. =. \nbss@iguanasuicide.net                     ((_/)o o(\\_))\nICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' \nhttp://iguanasuicide.net/                      \\_/     \n"}]}