4
\$\begingroup\$

This function formats a string by replacing every occurrence of '{n}' with the corresponding argument that was passed to the function where n is an index that starts at 0. The same index can be used multiple times and the order is irrelevant. If n is out of bounds or cannot be converted to an integer number I just add '{n}' to the result. The function is intended to be used e.g with config files that can be changed by the user where it would be unsafe to use sprintf. The use of std::ostringstream allows me to pass any type that has an overloaded << operator. I was considering using iterators in the loops but I'm not sure whether that would gain anything.

#pragma once

#include <string>
#include <vector>
#include <sstream>
#include <string_view>

namespace util::str{
    inline void to_string_vector(std::vector<std::string>&){} //Dummy used to end recursion

    template<typename T, typename ... Args>
    void to_string_vector(std::vector<std::string>& result, const T& t, Args&& ...args){
        result.push_back((std::ostringstream{} << t).str());
        to_string_vector(result, args...);
    }

    template<typename ... Args>
    std::string format(std::string_view fmt, Args&& ...args){
        std::string result;
        std::vector<std::string> values;

        result.reserve(fmt.size());
        to_string_vector(values, args...);

        for(std::size_t first = 0; first < fmt.size(); ++first){
            if(fmt[first] == '{'){
                for(std::size_t second = first + 1; second < fmt.size(); ++second){
                    if(fmt[second] == '}'){
                        try{
                            std::size_t index = std::stoi(std::string{fmt.substr(first + 1, second - first - 1)});

                            if(index < values.size())
                                result += values[index];
                            else
                                result += fmt.substr(first, second - first + 1);
                        }catch(...){
                            result += fmt.substr(first, second - first + 1);
                        }

                        first = second;

                        break;
                    }else if(fmt[second] == '{'){
                        result += fmt.substr(first, second - first);
                        first = second - 1;

                        break;
                    }
                }
            }else{
                result.push_back(fmt[first]);
            }
        }

        return result;
    }
}
\$\endgroup\$
  • \$\begingroup\$ The code could use some prettification (whitespaces) and you could possibly avoid creating the vector of strings by working directly with ostream, but it works and looks ok. Any concerns and/or future plans? \$\endgroup\$ – firda Sep 28 '18 at 23:53
  • \$\begingroup\$ @firda Thanks for the feedback. I’m not a big fan of too many whitespaces. I find them more annoying than helpful. Where would you suggest I add some more? I’m using the vector so that I can directly use the parsed index to look up the value. Not sure how I could do this directly with ostream. \$\endgroup\$ – Leon2806 Sep 29 '18 at 0:09
  • \$\begingroup\$ At least after } and before { when there is something else after/before: } else { and if(fmt[second] == '}') {, maybe after keywords (if (... and for () but that would be bonus :) \$\endgroup\$ – firda Sep 29 '18 at 0:14
  • \$\begingroup\$ As for the vector, well, such solution could be too advanced for you (e.g. capturing the arguments in a tuple). You can take a look at my own attempt (quite a different level). \$\endgroup\$ – firda Sep 29 '18 at 0:20
  • \$\begingroup\$ I see. A tuple is probably a better solution since I wouldn’t need the to_string_vector function at all then. \$\endgroup\$ – Leon2806 Sep 29 '18 at 0:36
3
\$\begingroup\$

As I already said in the comments, some white space would be good to add, especially around { and }. Then I mentioned that you could possibly avoid creating the vector of strings, but well, that's not that easy:

template<class... T> struct pack {
    static_assert(sizeof...(T) == 0, "Unmatched argument data");
    static constexpr size_t count = 0;

    template<class Action>
    bool exec(size_t pos, Action&& action = Action()) {
        return false;
    }
};
template<class T> struct pack<T> {
    static constexpr size_t count = 1;
    template<size_t I = 0> using type = std::enable_if_t<I == 0, T>;
    T value;
    pack() = default;
    template<class V>
    pack(V&& value) : value(std::forward<V>(value)) {}

    template<class Action>
    bool exec(size_t pos, Action&& action = Action()) {
        if (pos > 0) return false;
        action(value);
        return true;
    }
};
template<class T, class... Next> struct pack<T, Next...> {
    static constexpr size_t count = 1 + sizeof...(Next);
    template<size_t I = 0> using type = std::conditional_t<I == 0,
        T, typename pack<Next...>::template type<I == 0 ? 0 : I - 1>>;

    T value;
    pack<Next...> next;

    pack() = default;
    template<class First, class... Args>
    pack(First&& value, Args&&... args)
        : value(std::forward<First>(value))
        , next(std::forward<Args>(args)...) {}

    template<class Action>
    bool exec(size_t pos, Action&& action = Action()) {
        if (pos >= count) return false;
        if (pos == 0) action(value);
        else next.exec(pos - 1, std::forward<Action>(action));
        return true;
    }
};
template<class... Args>
inline auto make_pack(Args&&... args) {
    return pack<std::decay_t<Args>...>(std::forward<Args>(args)...);
}
template<class... Args>
std::string format(std::string_view fmt, Args&& ...args) {
    auto pack = make_pack(std::forward<Args>(args)...);
    std::ostringstream result;
    for (std::size_t at = 0; at < fmt.size();) {
        char c = fmt[at++];
        if (c == '{') {
            for (std::size_t start = at; at < fmt.size();) {
                c = fmt[at++];
                if (c == '}') {
                    try {
                        std::size_t index = std::stoi(std::string{ fmt.substr(start, at - 1 - start) });

                        if (index < pack.count)
                        {
                            pack.exec(index, [&result](auto it) { result << it; });
                            break;
                        }
                    }
                    catch (...) {
                    }
                    result << fmt.substr(start - 1, at - start + 1);
                    break;
                }
                if (c == '{') {
                    result << fmt.substr(start - 1, at - start + 1);
                    break;
                }
            }
        }
        else {
            result.put(c);
        }
    }
    return result.str();
}

I have also changed the indexing a bit, not that big of a difference. The pack is in no way perfect (reduced version of my data pack) and pack.exec(index, [&result](auto it) { result << it; }); is not easy for the compiler to optimize, but the machine code I have observed was not bad (two arguments, release build).

\$\endgroup\$
3
\$\begingroup\$

result.push_back((std::ostringstream{} << t).str()); does not compile with compilers that conform to the standard.

The operator<< actually returns a basic_ostream& which does not have a str() member. TT


We can use std::array instead of std::vector, and unpack the parameter pack directly into it:

template<class T>
std::string to_string(T&& t) {
    std::ostringstream sstr;
    sstr << t;
    return sstr.str();
}
...
auto values = std::array<std::string, sizeof...(Args)>{ to_string(std::forward<Args>(args))... };

Something to bear in mind: std::stoi is not very strict. It will parse something like { 1 267 hi there!} as 1 without batting an eyelid.

Using std::strtoul or std::strtoull would allow the character after the last converted character to be checked (and also avoid creating a copy of the number substring).

In fact, we could use std::strtoull to do the parsing after finding the first {. If the last converted character is }, we have a valid index. Otherwise, the characters can just be copied over.

(This may be too fiddly to be worth any gains in efficiency though (especially when using ostringstream for conversions), but it's something to consider).


The parsed index is checked to be < values.size(), but it may also be negative. Consider using result += values.at(index); which will throw a std::out_of_range error and end up in your catch block.

\$\endgroup\$
  • \$\begingroup\$ I didn't notice the std::ostream thing since it works with clang, gcc and msvc without any warnings so thanks for pointing that out :) I had a look at std::strtoul and it makes things significantly easier to write and read. I completely ditched the inner loop and now just check if the character that was parsed last by std::strtoul is '}'. \$\endgroup\$ – Leon2806 Sep 30 '18 at 16:38

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.