приемлемое исправление для большинства подписанных / неподписанных предупреждений? - PullRequest
9 голосов
/ 09 ноября 2008

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

Большинство мест, где это идет не так, - это простая итерация std :: vector, часто в прошлом это был массив, который позже был заменен на std :: vector. Таким образом, эти циклы обычно выглядят так:

for (int i = 0; i < someVector.size(); ++i) { /* do stuff */ }

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

for (unsigned i = 0; i < someVector.size(); ++i) { /*do stuff*/ }

Обычно это работает, но может молча прерываться, если цикл содержит какой-либо код, например, 'if (i-1> = 0) ...' и т. Д.

for (int i = 0; i < static_cast<int>(someVector.size()); ++i) { /*do stuff*/ }

Это изменение не имеет побочных эффектов, но делает цикл намного менее читабельным. (И это больше печатать.)

Итак, мне пришла в голову следующая идея:

template <typename T> struct vector : public std::vector<T>
{
    typedef std::vector<T> base;

    int size() const     { return base::size(); }
    int max_size() const { return base::max_size(); }
    int capacity() const { return base::capacity(); }

    vector()                  : base() {}
    vector(int n)             : base(n) {}
    vector(int n, const T& t) : base(n, t) {}
    vector(const base& other) : base(other) {}
};

template <typename Key, typename Data> struct map : public std::map<Key, Data>
{
    typedef std::map<Key, Data> base;
    typedef typename base::key_compare key_compare;

    int size() const     { return base::size(); }
    int max_size() const { return base::max_size(); }

    int erase(const Key& k) { return base::erase(k); }
    int count(const Key& k) { return base::count(k); }

    map() : base() {}
    map(const key_compare& comp) : base(comp) {}
    template <class InputIterator> map(InputIterator f, InputIterator l) : base(f, l) {}
    template <class InputIterator> map(InputIterator f, InputIterator l, const key_compare& comp) : base(f, l, comp) {}
    map(const base& other) : base(other) {}
};

// TODO: similar code for other container types

То, что вы видите, - это, в основном, классы STL с методами, которые возвращают size_type, переопределенный, чтобы возвращать просто 'int'. Конструкторы необходимы, потому что они не наследуются.

Что бы вы подумали об этом как о разработчике, если бы увидели подобное решение в существующей базе кода?

Не могли бы вы подумать: «да, они переопределяют STL, какой огромный WTF!», Или вы думаете, что это хорошее простое решение для предотвращения ошибок и повышения читабельности. Или, может быть, вы бы предпочли, чтобы мы потратили (половину) дня или около того на изменение всех этих циклов для использования std :: vector <> :: iterator?

(В частности, если это решение было объединено с запретом использования типов без знака для чего-либо, кроме необработанных данных (например, без знака) и битовых масок.)

Ответы [ 7 ]

7 голосов
/ 09 ноября 2008

Не выводить публично из контейнеров STL. У них есть не виртуальные деструкторы, которые вызывают неопределенное поведение, если кто-либо удаляет один из ваших объектов через указатель на базу. Если вы должны получить, например, из вектора, сделайте это в частном порядке и представьте части, которые вам нужно показать, с помощью using объявлений.

Здесь я бы просто использовал size_t в качестве переменной цикла. Это просто и читабельно. Постер, который прокомментировал, что использование индекса int выставляет вас как n00b, является правильным. Тем не менее, использование итератора для цикла по вектору представляет вас немного более опытным n00b - тем, кто не понимает, что оператор индекса для вектора имеет постоянное время. (vector<T>::size_type является точным, но ненужным многословным ИМО).

4 голосов
/ 09 ноября 2008

Хотя я не думаю, что «использовать итераторы, в противном случае вы смотрите n00b», это хорошее решение проблемы, но вывод из std :: vector выглядит гораздо хуже.

Во-первых, разработчики ожидают, что vector будет std: .vector, а map будет std :: map. Во-вторых, ваше решение не масштабируется для других контейнеров или для других классов / библиотек, которые взаимодействуют с контейнерами.

Да, итераторы уродливы, циклы итераторов не очень хорошо читаются, а typedefs только скрывают беспорядок. Но, по крайней мере, они действительно масштабируются и являются каноническим решением.

Мое решение? макрос для каждого. Это не без проблем (в основном это макрос, чёрт возьми), но оно понимает смысл. Это не так продвинуто, как, например, этот , но делает свою работу.

3 голосов
/ 22 августа 2013

Пропустить индекс

Самый простой подход - обойти проблему, используя итераторы, циклы на основе диапазона или алгоритмы:

for (auto it = begin(v); it != end(v); ++it) { ... }
for (const auto &x : v) { ... }
std::for_each(v.begin(), v.end(), ...);

Это хорошее решение, если вам на самом деле не нужно значение индекса. Он также легко обрабатывает обратные петли.

Используйте соответствующий тип без знака

Другой подход - использовать тип размера контейнера.

for (std::vector<T>::size_type i = 0; i < v.size(); ++i) { ... }

Вы также можете использовать std::size_t (из ). Есть те, кто (правильно) указывает, что std::size_t может не совпадать с типом std::vector<T>::size_type (хотя обычно это так). Однако вы можете быть уверены, что контейнер size_type поместится в std::size_t. Так что все в порядке, если вы не используете определенные стили для обратных циклов. Мой предпочтительный стиль для обратной петли такой:

for (std::size_t i = v.size(); i-- > 0; ) { ... }

С этим стилем вы можете безопасно использовать std::size_t, даже если он больше, чем std::vector<T>::size_type. Стиль обратных циклов, показанный в некоторых других ответах, требует приведения -1 к точно правильному типу и, следовательно, не может использовать более простой для ввода std::size_t.

Используйте подписанный тип (осторожно!)

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

#include <cassert>
#include <cstddef>
#include <limits>

template <typename ContainerType>
constexpr int size_as_int(const ContainerType &c) {
    const auto size = c.size();  // if no auto, use `typename ContainerType::size_type`
    assert(size <= static_cast<std::size_t>(std::numeric_limits<int>::max()));
    return static_cast<int>(size);
}

Теперь вы можете написать:

for (int i = 0; i < size_as_int(v); ++i) { ... }

Или обратные циклы традиционным способом:

for (int i = size_as_int(v) - 1; i >= 0; --i) { ... }

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

3 голосов
/ 09 ноября 2008

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

for (auto i = someVector.begin();
     i != someVector.end();
     ++i)
3 голосов
/ 09 ноября 2008

Я сделал это сообщество вики ... Пожалуйста, отредактируйте его. Я больше не согласен с советом против "int". Теперь я вижу, что это неплохо.

Да, я согласен с Ричардом. Вы никогда не должны использовать 'int' в качестве переменной подсчета в цикле, как те. Ниже показано, как вы можете создавать различные циклы с использованием индексов (хотя для этого нет особых причин, иногда это может быть полезно).

Форвард

for(std::vector<int>::size_type i = 0; i < someVector.size(); i++) {
    /* ... */
}

Назад

Вы можете сделать это, что идеально определяет поведение:

for(std::vector<int>::size_type i = someVector.size() - 1; 
    i != (std::vector<int>::size_type) -1; i--) {
    /* ... */
}

Вскоре, с появлением c ++ 1x (следующей версии C ++), вы можете сделать это так:

for(auto i = someVector.size() - 1; i != (decltype(i)) -1; i--) {
    /* ... */
}

Уменьшение значения ниже 0 приведет к тому, что я обернусь вокруг, потому что он без знака.

Но неподписанный вызовет ошибки в

Это никогда не должно быть аргументом, чтобы сделать это неправильно (использование 'int').

Почему бы не использовать std :: size_t выше?

Стандарт C ++ определяет в 23.1 p5 Container Requirements, что T::size_type, для T, являющейся некоторым Container, что этот тип является некоторой реализацией, определенной для целочисленного типа без знака. Теперь, используя std::size_t для i выше, ошибки будут тихо скрываться. Если T::size_type меньше или больше std::size_t, то оно будет переполнено i или даже не достигнет (std::size_t)-1, если someVector.size() == 0. Точно так же условие цикла было бы полностью нарушено.

0 голосов
/ 09 ноября 2008

Вы переосмысливаете проблему.

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

0 голосов
/ 09 ноября 2008

vector.size() возвращает size_t переменную, поэтому просто измените int на size_t, и все должно быть в порядке.

Ответ Ричарда более правильный, за исключением того, что для простого цикла много работы.

...