7
\$\begingroup\$

I'm writing an utility function that returns the extension(s) of a boost::filesystem::path (v3). Boost's path class already has some of this functionality

using 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"
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", 4)  -> ".vertex.shader.glsl.cache"
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", 5)  -> ".vertex.shader.glsl.cache"
/// extension("/usr/lib.cpp/file.vertex.shader.glsl.cache", 6)  -> ".vertex.shader.glsl.cache"
///
/// Edge cases:
/// extension("var/", 0)  -> ""
/// extension("var/", 1)  -> ""
/// extension("var/", 2)  -> ""
/// extension("var/file", 0)  -> ""
/// extension("var/file", 1)  -> ""
/// extension("var/file", 2)  -> ""
/// extension("var/file.", 0)  -> "."
/// extension("var/file.cpp.", 0)  -> ".cpp."
/// extension("var/file.cpp...abc..", 0)  -> ".cpp...abc.."
/// extension("var/file.cpp...abc..", 1)  -> "."
/// extension("var/file.cpp...abc..", 2)  -> ".."
/// extension("var/file.cpp...abc..", 3)  -> ".abc.."
/// extension("var/file.cpp...abc..", 4)  -> "..abc.."
///
////////////////////////////////////////////////////////////////////////////////
inline path extension( const path& path_, int dots = 0 ) {
    // Get the filename to ensure that some edge cases are dealt with. E.g.:
    // path{"/var/foo.bar/baz.txt"}.filename() -> path{"baz.txt"}
    auto filename = path_.filename();
    const auto& native = filename.native(); // Returns const std::wstring&

    // Reverse search for the nth dot
    auto nth_dot = algorithm::find_last_or_nth(native.crbegin(), native.crend(), '.', dots).base();
    // Compensate for reverse_iterator -> iterator conversion
    if (native.cbegin() != nth_dot) --nth_dot;

    return {nth_dot, native.cend()};
}

I've added additional examples in the comment block. I've cheated a bit and used path::filename() to make life easier. I know that this involves an additional copy. Note the function find_last_or_nth. This is an algorithm I've introduced to solve the problem. It is defined as follows:

////////////////////////////////////////////////////////////////////////////////
/// Find Last or nth
///
/// Let m be the number of occurences of value in the range [first, last).
///
///  1) If m > 0 and n >  0: Returns an iterator to the min(m, n)'th occurence of
///                          value in the range.
///  2) If m > 0 and n <= 0: Returns an iterator to the the m'th occurence of value 
///                          in the range.
///  3) If m = 0           : Returns last.
///
////////////////////////////////////////////////////////////////////////////////
template<typename input_iterator, typename T, typename counter>
input_iterator find_last_or_nth( input_iterator first, input_iterator last, const T& value, counter n = 0 ) {
    // Default to the m = 0 case
    auto result = last;
    // Loops until end of the range...
    // ...or n times if n > 0.
    if (0 < n) ++n;
    while (first != last && --n) {
        // Advance to the next occurence of value
        first = std::find(first, last, value);
        // Not yet end of range...
        if (first != last) {
            // ...so store last occurrence...
            result = first;
            // ...and advance to the remaining range.
            ++first;
        }
    }
    return result;
}

Have I missed some edge cases? What do you think of the style (I've tried to mimic STL and Boost)? Any alternative implementations?

\$\endgroup\$
  • \$\begingroup\$ I might quibble with your naming. There is only one file extension, the characters following the last period. Anything before that is not treated as an extension by the OS, it's just some naming convention you're using for your own purposes. \$\endgroup\$ – mattnewport Oct 31 '14 at 20:07
  • \$\begingroup\$ @mattnewport That's a good point! However, I can't find any official reference on the matter. The Wikipedia article on Filename extensions mentions that multiple extensions in Unix-like filesystems represent nested transformations. E.g. files.tar.gz. No source is given. A Google search for "multiple extensions" returns many related results. Again, not anything official. I don't know... Do you have a suggestion for a better name? Thanks for the edit, by the way. \$\endgroup\$ – Frederik Aalund Nov 1 '14 at 13:44
3
\$\begingroup\$

I agree with some of your API decisions but disagree with some others:

  • 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 < 0:

    • That's sort of garbage in, garbage out mentality, and I think not so good. Why would anyone in his right mind call this method with a negative value? Most likely the caller has a bug, and the value is unintentionally negative.
    • 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.

\$\endgroup\$
  • 1
    \$\begingroup\$ Great points. All of them! I'll change the default to dots = 1. I'll assert dots >= 0 to catch such errors in the debug build. Alternatively, I could just make dots an unsigned int. What do you think of that? Regarding the last point, I agree that it is somewhat non-obvious. Like you, I just found the behaviour to be reasonable. What could the alternative be? Throw an exception? I favor the current approach over that. I'll rename dots to max_dots as you suggest. Hopefully, the behaviour will become more obvious from that. I'll accept this answer. Cheers! \$\endgroup\$ – Frederik Aalund Nov 1 '14 at 14:03
  • \$\begingroup\$ Glad you liked it. Making dots an unsigned int won't prevent callers from misusing, so I think you really need the explicit check. The alternative for the last point could be providing two methods: one that returns exactly N extensions, and one that returns at most N, and let the method names indicate the difference clearly. But I think your method is good enough, the alternative might be overengineering. \$\endgroup\$ – janos Nov 1 '14 at 18:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.