Это использование оператора "," считается плохой формой? - PullRequest
28 голосов
/ 18 июля 2011

Я создал класс списка как средство замены переменных функций в моей программе, используемых для инициализации объектов, которые должны содержать изменяющийся список элементов. Класс списка имеет синтаксис использования, который мне действительно нравится. Однако я не видел, чтобы он использовался раньше, поэтому мне было интересно, не стоит ли использовать его только из-за этого факта? Базовая реализация класса list выглядит следующим образом ...

#include <list>
#include <iostream>
template<typename T>
struct list
{
    std::list<T> items;
    list(const list&ref):items(ref.items){}
    list(){}
    list(T var){items.push_back(var);}
    list& operator,(list add_){
        items.insert(items.end(),add_.items.begin(), add_.items.end());
        return *this;
    }
    list& operator=(list add_){
        items.clear();
        items.insert(items.end(),add_.items.begin(), add_.items.end());
        return *this;
    }
    list& operator+=(list add_){
        items.insert(items.end(),add_.items.begin(), add_.items.end());
        return *this;
    }
};

Это позволяет мне использовать это в коде так ...

struct music{
//...
};
struct music_playlist{
    list<music> queue;
//...
};
int main (int argc, const char * argv[])
{
    music_playlist playlist;
    music song1;
    music song2;
    music song3;
    music song4;
    playlist.queue = song1,song2; // The queue now contains song1 and song2
    playlist.queue+= song1,song3,song4; //The queue now contains two song1s and song2-4
    playlist.queue = song2; //the queue now only contains song2
    return 0;
}

Я действительно думаю, что синтаксис намного лучше, чем был бы, если бы я только что показал обычный контейнер stl, и даже лучше (и безопаснее), чем функции с переменным числом аргументов. Однако, поскольку я не видел этот синтаксис, мне любопытно, стоит ли мне его избегать, потому что, прежде всего, код должен быть легко понят другими программистами?

EDIT:

Вместе с этим вопросом я разместил вопрос , более нацеленный на решение актуальной проблемы.

Ответы [ 8 ]

40 голосов
/ 18 июля 2011

Почему бы не перегрузить оператор <<, как это делает QList? Тогда используйте это как:

playlist.queue << song1 << song2; // The queue now contains song1 and song2
playlist.queue << song1 << song3 << song4; //The queue now contains two song1s and song2-4
20 голосов
/ 18 июля 2011

Я согласен, что ваш синтаксис выглядит так, как вы его написали.
Моя основная проблема с кодом состоит в том, что я ожидал, что следующее будет таким же

playlist.queue = song1,song2;
playlist.queue = (song1,song2);  //more of c-style, as @Iuser notes.

тогда как на самом деле они совершенно разные.

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

//lets combine differnt playlists
new_playlist.queue =    song1        //the first playlist
                      ,(song3,song4) //the second playlist //opps, I didn't add song 3!
                      , song5;        //the third 

или

new_playlist.queue = (old_playlist.queue, song6); //opps, I edited my old playlist too!

Кстати, вы уже сталкивались с boost.assign: http://www.boost.org/doc/libs/1_47_0/libs/assign/doc/index.html

11 голосов
/ 18 июля 2011

Изменился ли приоритет в последнее время?

playlist.queue = song1,song2;

Это должно быть проанализировано как:

(playlist.queue = song1) , song2;

Ваши ',' и '+ =' совпадают!Было бы лучше семантическое соответствие, если бы ваш оператор запятой создал временный список, вставил левый и правый элементы и возвратил временный.Тогда вы могли бы написать это так:

playlist.queue = (song1,song2);

с явными паренами.Это дало бы C-программистам шанс на чтение кода.

7 голосов
/ 18 июля 2011

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

В отличие от Boost.Assign смешивание типов приводит к ошибке компиляции.

#include <boost/assign.hpp>

int main()
{
    int one = 1;
    const char* two = "2";
    list<int> li;
    li = one, two;

    using namespace boost::assign;
    std::list<int> li2;
    li2 += one, two;
}
5 голосов
/ 18 июля 2011

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

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

Я могу оценить Повышениепрецедент для такого рода вещей.Если вы уверены, что большинство людей, которые будут читать ваш код, также будут знакомы с Boost Assign, ваше собственное переопределение оператора может быть довольно разумным.Тем не менее, я бы посоветовал следовать предложению @ badzeppelin вместо использования оператора <<, как это делает iostreams.На каждого разработчика C ++ можно рассчитывать, что он столкнется с кодом, подобным: </p>

cout << "Hello world!"`

, и ваша операция добавления очень похожа на запись в поток.

5 голосов
/ 18 июля 2011

Это, вероятно, то, что принадлежит программистам, но вот мои два цента.

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

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

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

Делайте это на свой страх и риск.

4 голосов
/ 18 июля 2011

Это плохо на многих уровнях ...

Вы переопределяете list и следите за std::list.Большое нет-нет.Если вы хотите свой собственный класс списка - сделайте его с другим именем, не затеняйте стандартную библиотеку.

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

2 голосов
/ 18 июля 2011

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

list& operator,(list &add_){  // <--- pass by reference to avoid copies
  *this += add_;  // <--- reuse operator +=
  return *this;
}

Таким образом, вы всегда должны редактировать только operator +=, если вы хотите изменить логику.Обратите внимание, что мой ответ заключается в удобочитаемости и поддержке кода в целом .Я не буду беспокоиться о бизнес-логике, которую вы используете.

...