* [CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly
@ 2009-01-02 18:07 Johannes Schindelin
2009-01-02 18:28 ` Boyd Stephen Smith Jr.
0 siblings, 1 reply; 4+ messages in thread
From: Johannes Schindelin @ 2009-01-02 18:07 UTC (permalink / raw
To: git, gitster
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 <johannes.schindelin@gmx.de>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly
2009-01-02 18:07 [CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly Johannes Schindelin
@ 2009-01-02 18:28 ` Boyd Stephen Smith Jr.
2009-01-02 18:49 ` Johannes Schindelin
0 siblings, 1 reply; 4+ messages in thread
From: Boyd Stephen Smith Jr. @ 2009-01-02 18:28 UTC (permalink / raw
To: Johannes Schindelin; +Cc: git, gitster
[-- Attachment #1: Type: text/plain, Size: 969 bytes --]
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/ \_/
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly
2009-01-02 18:28 ` Boyd Stephen Smith Jr.
@ 2009-01-02 18:49 ` Johannes Schindelin
2009-01-02 20:04 ` Boyd Stephen Smith Jr.
0 siblings, 1 reply; 4+ messages in thread
From: Johannes Schindelin @ 2009-01-02 18:49 UTC (permalink / raw
To: Boyd Stephen Smith Jr.; +Cc: git, gitster
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly
2009-01-02 18:49 ` Johannes Schindelin
@ 2009-01-02 20:04 ` Boyd Stephen Smith Jr.
0 siblings, 0 replies; 4+ messages in thread
From: Boyd Stephen Smith Jr. @ 2009-01-02 20:04 UTC (permalink / raw
To: Johannes Schindelin; +Cc: git, gitster
[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]
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/ \_/
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-01-02 20:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-02 18:07 [CLEANUP PATCH RESEND] git wrapper: Make while loop more reader-friendly Johannes Schindelin
2009-01-02 18:28 ` Boyd Stephen Smith Jr.
2009-01-02 18:49 ` Johannes Schindelin
2009-01-02 20:04 ` Boyd Stephen Smith Jr.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).