В C++ строка форматирования опять Часть-1


Ранее спрашивал здесь.

Код теперь доступен на Гитхабе.

После предыдущего обзора я добавил юнит-тесты.

Поскольку он большой, он будет приходить на пару частей.

Часть 1 | Часть 2 | Часть 3 | Часть 4

Часть 1

Это интерфейс. Это позволяет передать строку формата и кучу аргументов, чтобы быть отформатирован. Если в результате этого затем применяется к потоку, он будет печатать значение.

Формат.ч

#ifndef THORSANVIL_IOUTIL_FORMAT_H
#define THORSANVIL_IOUTIL_FORMAT_H

#include "printToStream.h"
#include "Formatter.h"

#include <ostream>
#include <sstream>
#include <string>
#include <vector>
#include <exception>
#include <stdexcept>

namespace ThorsAnvil::IOUtil
{

template<typename... Args>
class Format
{
    std::string                     format;
    std::tuple<Args const&...>      arguments;
    std::vector<std::string>        prefixString;
    std::vector<Formatter>          formater;
    public:
        Format(char const* fmt, Args const&... args)
            : format(fmt)
            , arguments(args...)
        {
            std::size_t count       = sizeof...(args);
            std::size_t pos         = 0;
            Dynamic     dynamicSize = Dynamic::None;
            for (std::size_t loop = 0; loop < count; ++loop)
            {
                // Find the prefix string before the next % sign (this may be empty)
                std::pair<std::string, std::size_t> prefix = getNextPrefix(format, pos, [](std::size_t p){return p == std::string::npos;}, "not enough format");
                pos += prefix.second;
                prefixString.emplace_back(std::move(prefix.first));

                // Now that I have found the %
                // Convert the next part of the string into a Formatter object.
                formater.emplace_back(format.data() + pos, dynamicSize);
                pos         += formater.back().size();

                // Take into account the Dynamic Width/Precesion prevent the format from being read.
                // Note we can have Width/Precision or Both.
                Dynamic     newDynamic  = formater.back().isDynamicSize();
                dynamicSize = (newDynamic == Dynamic::Precision && dynamicSize == Dynamic::Width) ? Dynamic::Both : newDynamic;
            }
            // After the last formatter check for a last fixed string (this may be empty)
            // But there better not be any more % signs as we don't have any parameters left for them.
            std::pair<std::string, std::size_t> prefix = getNextPrefix(format, pos, [](std::size_t p){return p != std::string::npos;}, "too many format");
            pos += prefix.second;
            prefixString.emplace_back(std::move(prefix.first));
        }
        // Using the operator<< is the same as calling print on the object.
        friend std::ostream& operator<<(std::ostream& s, Format const& format)
        {
            format.print(s);
            return s;
        }
        void print(std::ostream& s) const
        {
            doPrint(s, std::make_index_sequence<sizeof...(Args)>());
        }
    private:
        // This finds the next '%' taking into account that %% is a not a scan token.
        std::pair<std::string, std::size_t> getNextPrefix(std::string const&, std::size_t pos, std::function<bool(std::size_t)>&& test, char const* mes)
        {
            std::string prefix;
            std::size_t extra = 0;
            std::size_t nextFormatter = format.find('%', pos);
            while (nextFormatter != std::string::npos && format[nextFormatter + 1] == '%')
            {
                nextFormatter = format.find('%', nextFormatter + 2);
            }
            prefix += format.substr(pos, nextFormatter - pos);
            prefix.erase(std::remove_if(std::begin(prefix), std::end(prefix), [first = false](char val) mutable
            {
                if (val == '%')
                {
                    first = !first;
                    return !first;
                }
                return false;
            }), std::end(prefix));
            if (test(nextFormatter))
            {
                std::stringstream message;
                message << "Invalid Format: " << mes << " specifiers for provided arguments";
                throw std::invalid_argument(message.str());
            }
            return {prefix, prefix.size() + extra};
        }
        // For each argument we pass
        // Pass a prefix string (might be empty) the formatter then the argument.
        template<std::size_t I>
        std::ostream& printValue(std::ostream& s) const
        {
            return s << prefixString[I] << formater[I] << std::get<I>(arguments);
        }

        // Print all the values in order by calling printValue() on each parameter.
        // Then print the final fixed string (might be empty)
        template<std::size_t... I>
        void doPrint(std::ostream& s, std::index_sequence<I...> const&) const
        {
            std::ostream* ignore[] = {&printValue<I>(s)...};
            s << prefixString.back();
        }

};

template<typename... Args>
Format<Args...> make_format(char const* fmt, Args const&... args)
{
    return Format<Args...>(fmt, args...);
}

}

#endif

Использование:

int main()
{
    std::cout << make_format("Test One\n");
    std::cout << make_format("Test Two   %d\n", 12);
    std::cout << make_format("Test Three %d %f\n", 12, 4.56);
    std::cout << make_format("Test Four  %d %d\n", 12, 4.56); // Should throw
}


Комментарии
2 ответа

Прежде всего, это очень хороший и чистый код! Тем не менее, у меня есть некоторые вещи, чтобы придираться на:

Форматирование Кода

Давайте поговорим о длине линии. По состоянию на сейчас, у вас есть (физической) линии в программе весом в целых 160 символов.

Прежде всего, это достаточно легко сделать блоки кода на код-ревью есть горизонтальная полоса прокрутки, что непрактично и не очень красиво.

Во-вторых, до 160 знаков-это слишком много для многих из нас, чтобы справиться в обычной среде разработки. Может быть, у вас есть хороший, большой монитор или два, позволяющие открывать одновременно четыре файла на ширину линии 200 или больше. Впрочем, я, конечно, не, и заставляет меня работать с вашей программой на моем 14" ноутбук с диагональю около 200 символов ширина линии на весь экран не очень любезен с вами.

Теперь, правильная максимальная ширина линии была предметом непрекращающихся споров. Я сторонник старых добрых 80 знаков стандартные, но я бы утверждать, что ничего вокруг 100 символов-это нормально. Однако, на мой взгляд, 160-это не так; и вы, вероятно, нарушить некоторые люди, придерживаясь ее.

Другой вопрос, что я вижу-это по горизонтали. Например, давайте взглянем на две строчки в коде:

pos         += formater.back().size();

Dynamic newDynamic = formater.back().isDynamicSize();

Оба они могут быть найдены в теле Format конструктор. Очевидно, вы пытаетесь выровнять определения/операторов. Еще более очевидно, что вы не на него. Хотя обе эти линии совпадают с чем-то, что они соответствуют не всем легко распознать. Я не знаю, является ли это по вине вкладка-ширина несоответствие между блоком кода и исходный код. Все что я могу сказать, что следы оставлены не на месте, и оставил меня в замешательстве первый раз, когда я замазывал свой код.

В общем, выравнивая инициализаторы и т. д. вполне приемлемо, но вы должны воздерживаться от этого, если ссылка выравнивание нет в большинстве одну линию. В противном случае, он просто будет выглядеть странно, и в целом программиста так хорошо, как вы недостойно.

О char const*

Ах, c-стринги. Не использовать их, если вы наверняка придется. Почему? Потому что...


  1. Они по своей сути небезопасно. Вы не можете гарантировать, что вы получаете указатель является указателем на действительный, оканчивающуюся нулем строку, а не все остальное, как набор двоичных данных, или вообще в никуда.

  2. C-стринги-это дурость. Они не содержат данные о длине, ни чего-либо еще, кроме адреса их содержание. Как правило, вы тратите здесь на вызова strlen на шнурке, длина которого теоретически известен во время компиляции.

Какие альтернативы? Поскольку вы, кажется, на C++17, это идеальный вариант использования для std::string_view. Если это не так, то рекомендации предлагают библиотеку различных струнных пролетов. Если оба этих варианта будут заблокированы для вас, вы могли бы шаблон свой выход на тип параметра. Как последний вариант, вы могли бы также просто взять std::string const& или std::string&&.

О getNextPrefix

В целом, этот метод кажется мне немного непонятно. Начиная с определения, зачем тебе анонимный первого параметра? Какой в этом смысл? Это мне кажется весьма сомнительной. Есть только несколько причин, чтобы иметь такой параметр в любом месте, в первую очередь, и никто из них, кажется, применить здесь. Это способ выполнить требования интерфейс, что вы не сказали нам? Кажется маловероятным, поскольку он является частным.

Следующим, какие Франкенштейн строка mes? Когда я читал Format конструктор, я уже совсем запуталась сначала, потому что эти последние строки в вызовы этот метод, кажется, не делает много смысла. На самом деле, эти строки имеют смысл только в контексте getNextPrefix.

Мне кажется, что вы попали в внезапного случае шаблон дизайна безумие: оценка сухой принцип превыше всего, вы уменьшили эти две строки сообщений до минимума, разделить каждый общий делитель из. Если это был обычный код, который будет скорее всего нормально, но мы говорим здесь о струны.

Даже если этот способ является частным и ни один пользователь никогда не будет иметь дело с этой причудливой интерфейс, подумайте о том, что вы на самом деле делаете, когда вы определяете подходящий способ. Вы не абстрагируясь от некоторой функциональности за имя, и это имя теперь должен передать ее смысл там, где его заменяет код, который он содержит. В ваш код, однако, ничего не выражало, (что вовсе не очевидно), что только части приговора должны быть передан mes.

В общем, разделение того, что логически принадлежат к одной части строки (например, предложение, фразу, имя и так далее. в зависимости от контекста) почти никогда не является хорошей идеей. Вы, конечно, снижается читабельность, на усиление несколько символов сохранен код.

Другое дело я не большой поклонник используя числовые индексы, а не итераторы. Поступая так, вы будете получать очень мало, но уменьшает семантическую целостность своего кода. Ведь std::size_t не очень полезна имя. Все это говорит, "Вот это беззнаковым числом", а std::string::iterator выражает "такое вот положение в std::string".

Итераторы имеют некоторые недостатки, такие как возможность признания недействительными в случае, если основной контейнер перемещается, но те здесь не действуют, поскольку все, что вы делаете, которые могут быть написаны с использованием итераторов одного логического действия.

Это позволит также избавиться от (возможно) некрасиво std::string::nposи некрасиво find метод std::string (должны быть заменены std::find).

Один последний пункт: std::function довольно тяжелая реализации мудрый. Хотя это, в общем, не проблема дизайн на всех, чтобы использовать его (и я хотел бы пойти дальше и сказать, что это еще и хорошая практика), он может иметь довольно тяжелые стопки и недостатки исполнения. Если вы не возражаете тех, Вы можете перейти на его использование. Если вы делаете, вы должны сделать типа test шаблон параметр, или сходить с пути прохождения простой указатель на функцию (что гораздо хуже, стиль-мудрый, а лучше для кода, размер).

Игнорирование Ценностей

std::ostream* ignore[] = {&printValue<I>(s)...};

это довольно некрасиво. На мой взгляд,

static_cast<void>(printValue<I>(s)...);

выглядит намного приятнее.

Включает в себя

Вам не хватает включать на std::size_t. Желательно, что бы #include <cstddef>Хотя существует как минимум пять других заголовков гарантированно определить его. Аналогично, вам не хватает #include <utility> для std::move, std::pair и все std::integer_sequence пакет, и #include <functional> для std::function. Наконец, у вас тоже хватает #include <algorithm> для std::remove_if, #include <iterator> для std::begin и std::endи #include <tuple> за все std::tuple-связанные. Пожалуйста, будьте более осторожны вокруг включает!

Другое дело, что меня слегка беспокоит, что ваш включать не сортируются в алфавитном порядке. Хотя это не проблема по сути для такого маленького проекта, как ваш, он может расти, чтобы быть довольно неприятностью в крупных проектах с десятками или более включает. Ваш заказ включает в себя от A до Z делает обзор их гораздо легче и помогает проверить, все ли имеется на самом деле.

Опечатки и орфографические несоответствия

Наконец, самый педантичный и наименее полезный код комментарий смысл всего!

Format содержит переменную-член полбу formater. Однако, в конструкторе, у вас есть комментарий

// Convert the next part of the string into a Formatter object.

Поскольку "праматерия" с двумя 'T' - это правильное написание слова, вы, возможно, захотите изменить имя переменной.

Еще один комментарий твой читает

// Take into account the Dynamic Width/Precesion prevent the format from being read.

Я предполагаю, что вы имели в виду "точность" здесь.

В общем, я хотел бы призвать вас, чтобы сделать больше использовать запятые и знаки препинания вообще запрещено свои комментарии. Например:

// For each argument we pass
// Pass a prefix string (might be empty) the formatter then the argument.

было бы более правильным (с точки зрения языка), если написано как

// For each argument we pass:
// Pass a prefix string (might be empty), the formatter, then the argument.

Я лично считаю, что это очень важно, чтобы не быть небрежным с кодом (как я предполагаю, что вы делаете, так как если вы этого не сделали, вы бы не пришли сюда для просмотра кода), который включает комментарии. Причина в том, что вводя разгильдяйства в одной точке, как правило переносятся к другим частям работы, по крайней мере для меня. Более важно, однако, что комментарии-это способ общения с другими программистами; как таковые, они должны быть четкими и легко понять, что несколько мешает плохой орфографии и грамматики.


Этот ответ получился какой-то массивной стены слов. Большинство пунктов, которые я перечислил, основанные на мнении, в определенной степени, поэтому, пожалуйста, большинство таких, как 'Прошу считать...' вместо 'сделаем!'. Однако, есть один момент, который делает этот императив природы, первый пункт упоминается под включает в себя. Если есть только один вещь, вы считаете от этого ответа в целом, пожалуйста, ссылки, так как это представляет серьезную угрозу для действия вашей программы.

7
ответ дан 4 марта 2018 в 11:03 Источник Поделиться

Код

Бен Стефан отмечает отсутствующих включает и множество других вопросов, так что я просто добавить / подтвердить следующее:


  • doPrint не компилируется с индекса MSVC 2015 года в связи с созданием нулевого размера массива. Он должен быть:

    std::ostream* ignore[] = { nullptr, &printValue<I>(s)... };

  • (Ошибка:) extra в getNextPrefix() только инициализируется в ноль, и не изменилось. Я предполагаю, что это предназначается, чтобы отслеживать любые %s убрал из префикса (так что %%% будет работать правильно - сейчас это не делает).

  • getNextPrefix() это действительно сбивает с толку: безымянный параметр. Передавая в функцию "проверить" очередного форматирования здесь.


Дизайн

Вопрос на миллион долларов: если это успешно подтверждает printf аргументы... почему бы просто не назвать printf?


В целом (и как Incomputable ответ на предыдущий комментарий отметить) printf форматирование не подходит для C++. Мы знаем, что типы, которые должны быть напечатаны, так что все типы, указанные в строке формата являются избыточными. Это также получается скомпилировать проблема времени в ошибки во время выполнения.

В стиле C#формат строки, вероятно, подходит лучше.


(Еще один момент из Incomputable предыдущего ответа): конструкция в настоящее время является частью пути между функцией и многоразовые класс. Хранение аргументы внутри класса с помощью ссылки подвержена ошибкам).

Я предлагаю либо:


  • Функция (в этом случае класс формат должен быть невидимым и недоступным).

  • Многоразовый объект, который принимает строку формата в конструкторе и разбирает его (как Format ныне), а затем позволяет поток оператора (или эквивалентная) функция должна вызываться с разными аргументами любое количество раз.


s << prefixString[I] << formater[I] << std::get<I>(arguments) силами одной строки префикса для каждого Formatterи один Formatter для каждого аргумента value.

Это 1:1:1 требование является некорректным, из-за способа * аргументы для ширины и точности работы. Снятие этого требования позволит следующее:


  • prefixStringмогут быть сохранены с std::string_view или итераторы, вместо строковых копии.

  • Нет необходимости в сложных логических отслеживание динамических размеров при разборе в Format / Formatter конструкторы.

  • Нет необходимости saveToStream.

Это может быть сделано путем слияния formater и prefixstring хранение контейнеров в один вариант. Тогда мы можем пройтись объединенный контейнер (во время выполнения), наряду с аргументами (статически).

Очень грубо говоря, что-то вроде следующего:

#include "Variant.h" // use std::variant instead...

#include <cassert>
#include <vector>
#include <string>

struct StringPrimitive
{
std::string::const_iterator start, end; // use std::string_view!
};

struct FormatSpecifier
{
// ... TODO: take actual width / precision
bool NeedsWidth() const { return !m_widthSet; }
bool NeedsPrecision() const { return !m_precisionSet; }

void SetWidth(int) { m_widthSet = true; }
void SetPrecision(int) { m_precisionSet = true; }

bool m_widthSet = false;
bool m_precisionSet = false;

// ... TODO: everything else!
};

// nicked from a stackoverflow post:
template <size_t N, typename... Args>
decltype(auto) magic_get(Args&&... as) noexcept {
return std::get<N>(std::forward_as_tuple(std::forward<Args>(as)...));
}

struct Formatter
{
std::string m_formatString;

using PrimitiveT = Variant<StringPrimitive, FormatSpecifier>; // use std::variant instead...
using PrimitivesT = std::vector<PrimitiveT>;
PrimitivesT m_primitives;

explicit Formatter(std::string const& formatString):
m_formatString(formatString)
{
// ... TODO: parse the format string properly!

m_primitives.push_back(PrimitiveT(StringPrimitive{ m_formatString.begin(), m_formatString.begin() + 13u }));
m_primitives.push_back(PrimitiveT(FormatSpecifier()));
}

template<std::size_t Index, class... Args>
void Print(std::true_type, std::ostream& os, PrimitivesT::iterator p, Args&&... args) // no more args to process
{
if (p == m_primitives.end()) // done! end the "recursion"
return;

if (p->IsType<StringPrimitive>())
{
auto& string = p->Get<StringPrimitive>();

os << std::string(string.start, string.end); // ... TODO: print string held in StringPrimitive

++p; // consume primitive

Print<Index>(std::true_type(), os, p, args...);
}
else if (p->IsType<FormatSpecifier>())
{
throw std::runtime_error("extra format specifier in string, or missing argument(s)!");
}
else
{
assert(false); // uhh...
}
}

template<std::size_t Index, class... Args>
void Print(std::false_type, std::ostream& os, PrimitivesT::iterator p, Args&&... args)
{
if (p == m_primitives.end())
throw std::runtime_error("missing format specifier in string, or extra argument(s)!");

if (p->IsType<StringPrimitive>()) // or some kind of visit function...?
{
auto& string = p->Get<StringPrimitive>();

os << std::string(string.start, string.end); // ... TODO: print string held in StringPrimitive

++p; // consume primitive

Print<Index>(std::false_type(), os, p, args...);
}
else if (p->IsType<FormatSpecifier>())
{
auto& formatSpecifier = p->Get<FormatSpecifier>();

if (formatSpecifier.NeedsWidth())
{
formatSpecifier.SetWidth(magic_get<Index>(args...));
}
else if (formatSpecifier.NeedsPrecision())
{
formatSpecifier.SetPrecision(magic_get<Index>(args...));
}
else
{
os << magic_get<Index>(args...); // ... TODO: print value at index using the format specifier

++p; // consume primitive
}

Print<Index + 1>(std::integral_constant<bool, (sizeof...(Args) == Index + 1)>(), os, p, args...);
}
else
{
assert(false); // uhh...
}
}

template<class... Args>
std::ostream& operator()(std::ostream& os, Args&&... args)
{
Print<0>(std::integral_constant<bool, (sizeof...(Args) == 0)>(), os, m_primitives.begin(), args...);

// ... TODO: clear all the dynamic widths / precisions in the format specifiers

return os;
}
};

#include <iostream>

int main()
{
auto f = Formatter("test string %%%*.*d\n");

f(std::cout, 5, 5, 12);

std::cout << std::endl;

f(std::cout, 1278); // 1 arg because we didn't clear the dynamic widths / precisions... this should throw

std::cout << std::endl;
}

1
ответ дан 6 марта 2018 в 09:03 Источник Поделиться