On 03/04/2014 10:40 AM, David Kastrup wrote:
> Michael Haggerty <mhag...@alum.mit.edu> writes:
> 
>> The beginning of the loop ensures that slash can never be NULL.  So
>> don't keep checking whether it is NULL later in the loop.
>>
>> Furthermore, there is no need for an early
>>
>>     return it;
>>
>> from the loop if slash points at the end of the string, because that
>> is exactly what will happen when the while condition fails at the
>> start of the next iteration.
> 
> Hm.  Another suggestion.  You have
> 
>               const char *slash = strchr(path, '/');
>               if (!slash)
>                       slash = path + strlen(path);
> [...]
>               sub = find_subtree(it, path, slash - path, 0);
> [...]
>               path = slash;
>               while (*path == '/')
>                       path++;
>       }
> 
> At the price of introducing another variable, this could be
> 
>               const char *slash = strchr(path, '/');
>               size_t len = slash ? slash - path : strlen(path);
> [...]
>               sub = find_subtree(it, path, len, 0);
> [...]
>                 if (!slash)
>                       break;
>               for (path = slash; *path == '/';)
>                       path++;
>       }
> 
> This introduces another variable and another condition.  The advantage
> is that "slash" indeed points at a slash or is NULL, so the variable
> names correspond better to what happens.  Alternatively, it might make
> sense to rename "slash" into "end" or "endpart" or whatever.  Since
> I can't think of a pretty name, I lean towards preferring the latter
> version as it reads nicer.  I prefer code to read like children's books
> rather than mystery novels.

I think we're reaching the point of diminishing returns here.  I can't
muster a strong feeling either way about your suggestion to add a "len"
variable.

BTW, I purposely didn't use a "for" loop at the end (even though I
usually like them) because I wanted to keep it prominent that path is
being updated to the value of slash.  Putting that assignment in a for
loop makes it easy to overlook because it puts "path" in the spot that
usually holds an inconsequential iteration variable.

YMMV.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to