Michael Haggerty [off-list ref] 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.
--
David Kastrup
On 03/04/2014 10:40 AM, David Kastrup wrote:
Michael Haggerty [off-list ref] writes:
quoted
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
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/