Hi, On Sat, 22 Jul 2017, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: > > > But this whole thread taps into a gripe I have with parts of Git's code > > base: part of the code is not clear at all in its intent by virtue of > > calling whatever POSIX function may seem to give the answer for the > > intended question, instead of implementing a function whose name says > > precisely what question is asked. > > > > In this instance, we do not call a helper get_file_size(). Oh no. That > > would make it too obvious. We call lstat() instead. > > I agree with you for this case and a case like this in general. > > In codepaths at a lot lower level (they tend to be the ancient and > quite fundamental ones) in our codebase, lstat() is often directly > used by the caller because they are interested not only in a single > aspect of a path but many fields in struct stat are of interest. > > When the code is interested in existence or size or whatever single > aspect of a path and nothing else, however, the code would become > easier to read if a helper function with a more specific name is > used. And it may even help individual platforms that do not want to > use the full lstat() emulation, by telling them that other fields in > struct stat are not needed. > > Of course, then the issue becomes what to do when we are interested > in not just one but a selected few attributes. Perhaps we create a > helper "get_A_B_and_C_attributes_for_path()", which may use lstat() > on POSIX and the most efficient way to get only A, B and C attributes > on non-POSIX platforms. The implementation would be OK, but the naming > becomes a bit hard; we need to give it a good name. > > Things gets even more interesting when the set of attributes we are > interested in grows by one and we need to rename the function to > "get_A_B_C_and_D_attributes_for_path()". When it is a lot easier to > fall back to the full lstat() emulation on non-POSIX platforms, the > temptation to just use it even though it would grab attributes that > are not needed in that function grows, which needs to be resisted by > those who are doing the actual implementation for a particular platform. It becomes a lot easier to fall back to lstat(), if a lot less readable, yes. Until, that is, one realises that the function name does not have to encode what information is sought. It can be a bit field in a parameter instead. There are even precendents in Git's own source code for that rather smart paradigm. Ciao, Dscho
