git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.

From
Junio C Hamano <junkio@cox.net>
Date
Sep 27, 2006, 06:19 UTC
Message-ID
<7vejtxhlv6.fsf@assigned-by-dhcp.cox.net>
In-Reply-To
<Pine.LNX.4.64N.0609262216390.12560@attu2.cs.washington.edu>
David Rientjes <rientjes@cs.washington.edu> writes:
> When I read "x > 0", my mind parses that very easily.  When I read "0 < 
> x", it takes me a few cycles longer.  I think the goal of any software 
> project is to not only emit efficient and quality code, but also code that 
> can be read and deciphered with ease unless it's impossible otherwise.

Well, the thing is, I end up being the guy who needs to stare at git code longer than you do ;-).

Before --stat-width was introduced there was code like this:
	if (max + len > 70)
		max = 70 - len;

Here "len" is the width of the filename part, and "max" is the number of changes we need to express. The code is saying "if we use one column for each changed line, does graph and name exceed 70 columns -- if so use the remainder of the line after we write name for the graph". Your "constant at right" rule makes this kosher.

If we make that to a variable, say line_width, we can still write:

	if (max + len > line_width)
        	...

I however tend to think "if line_width cannot fit (max + len) then we do this", which would be more naturally expressed with:

	if (line_width < max + len)
        	...

Now, at this point, it is really the matter of taste and there is no real reason to prefer one over the other. Textual ordering lets my eyes coast while reading the code without taxing the brain. I can see that the expression compares two quantities, "line_width" and "max + len", and the boolean holds true if line_width _comes_ _before_ "max + len" on the number line (having number line in your head helps visualizing what is compared with what). If you write the comparison the wrong way, it forces me to stop and think -- because on my number line smaller numbers appear left, and cannot help me reading the comparison written in "a > b" order.

I could try writing constants on the right hand side when constants are involved, but I do not think it makes much sense. It means that I would end up doing:

- if (max + len > 70) - max = 70 - len; + if (line_width < max + len) + max = line_width - len;

Consistency counts not only while reading the finished code, but also it helps reviewing the diff between the earlier version that used constant (hence forced to have it on the right hand side by your rule) and the version that made it into a variable.

> To change the code itself because of a hard 80-column limit or because 
> you're tired of hitting the tab key is poor style.

Well, the program _firstly_ matches the logic flow better, and _in_ _addition_ if you write it another way it becomes unnecessarily too deeply indented. So while I agree with you as a general principle that indentation depth should not dictate how we code it does not apply to this particular example.

Previous: David RientjesNext: David Rientjes
Message 5 of 18 in “diff --stat: sometimes use non-linear scaling.”
  1. 3/3 diff --stat: sometimes use non-linear scaling.Junio C Hamano, Sep 27, 2006
  2. David RientjesSep 27, 2006
  3. Junio C HamanoSep 27, 2006
  4. David RientjesSep 27, 2006
  5. Junio C HamanoSep 27, 2006
  6. David RientjesSep 27, 2006
  7. Junio C HamanoSep 27, 2006
  8. David RientjesSep 27, 2006
  9. Johannes SchindelinSep 27, 2006
  10. Petr BaudisOct 6, 2006
  11. Johannes SchindelinSep 27, 2006
  12. Martin WaitzSep 27, 2006
  13. Linus TorvaldsSep 27, 2006
  14. Martin WaitzSep 28, 2006
  15. Junio C HamanoSep 28, 2006
  16. Andreas EricssonSep 29, 2006
  17. Johannes SchindelinSep 27, 2006
  18. SeanSep 27, 2006

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.