patterncppMinor
Get extension(s) from path
Viewed 0 times
fromgetpathextension
Problem
I'm writing an utility function that returns the extension(s) of a
Note that the
I propose the utility function:
where
Note that
Here is my proposed
```
////////////////////////////////////////////////////////////////////////////////
/// Extension
///
/// Negative or zero "dots" value returns all extensions. E.g.:
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", -2) -> ".vertex.shader.glsl.cache"
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", -1) -> ".vertex.shader.glsl.cache"
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", 0) -> ".vertex.shader.glsl.cache"
///
/// Positive "dots" value returns at most "dots" extensions (counting from the end of the path). E.g.:
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", 1) -> ".cache"
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", 2) -> ".glsl.cache"
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", 3) -> ".shader.glsl.cache"
boost::filesystem::path (v3). Boost's path class already has some of this functionalityusing path = boost::filesystem::path;
path shader_file{"/var/.private/code/main.vertex.glsl"};
shader_file.extension(); // returns ".glsl"Note that the
. is included. However, path's extension() function only returns the last extension. I can't get .vertex.glsl returned.I propose the utility function:
inline path extension( const path& p, int dots )where
dots indicates how many extensions should be returned:extension(shader_file, 1); // returns ".glsl"
extension(shader_file, 2); // returns ".vertex.glsl"
extension(shader_file, 42); // returns ".vertex.glsl"Note that
dots may exceed the actual number of extensions (all extensions are just returned in this case). Getting all extensions is a common use case. Setting dots arbitrarily high seems wrong. Therefore, I define that for 0 >= dots all extensions are returned.extension(shader_file, 0); // returns ".vertex.glsl"
extension(shader_file, -356); // returns ".vertex.glsl"Here is my proposed
extension function:```
////////////////////////////////////////////////////////////////////////////////
/// Extension
///
/// Negative or zero "dots" value returns all extensions. E.g.:
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", -2) -> ".vertex.shader.glsl.cache"
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", -1) -> ".vertex.shader.glsl.cache"
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", 0) -> ".vertex.shader.glsl.cache"
///
/// Positive "dots" value returns at most "dots" extensions (counting from the end of the path). E.g.:
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", 1) -> ".cache"
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", 2) -> ".glsl.cache"
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", 3) -> ".shader.glsl.cache"
Solution
I agree with some of your API decisions but disagree with some others:
-
-
Return all extensions when
-
Return all extensions when
Other than these API considerations, the implementation seems fine to me.
-
dots = 0 by default: this contradicts with the most common meaning of "extension" is everything from the last dot to the end. So I would change the default to 1. That way the behavior will be consistent with boosts' own extension method, so possibly less confusing to users.-
Return all extensions when
dots = 0: I agree, this seems reasonable and natural.-
Return all extensions when
dots
- It would be better to refuse to work with such invalid values. That will help the caller realize early that something is wrong. If the parameter variable in the caller's side unintentionally ends up having a negative value, it's good to give a clue to the caller early, rather than trying to handle this gracefully, covering up a possible bug. It's easiest to fix bugs close to their source. Handling bad behavior of the caller gracefully is hardly the responsibility of this function, and you will serve the caller better by giving a clue.
- An API is better when there is one way to do something rather than many, which would make the caller ask questions like "which way is better?", "is there a difference if I do like this and this?". It's better to be unambiguous and simple. Treat only 0 as special, no need to extend that concept to negative values.
-
Return at most n extensions, when there are less than that, return all:
I don't like this rule because it's not obvious from the method signature. I would have to look at the documentation to know what to expect. But I suppose this is a reasonable behavior, and practical this way. Perhaps if you rename the parameter to maxDots` then it will be sufficient hint that the method may return fewer extensions than requested.Other than these API considerations, the implementation seems fine to me.
Context
StackExchange Code Review Q#68459, answer score: 3
Revisions (0)
No revisions yet.