Templated string formatting

Question

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;
    }
}

Answer 1

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).

Answer 2

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.