Это слишком умно или небезопасно? - PullRequest
13 голосов
/ 17 мая 2010

Недавно я работал над некоторым кодом и решил поработать над перегрузкой моего оператора в c ++, потому что я никогда раньше не реализовывал его.Поэтому я перегружал операторы сравнения для моего класса матрицы, используя функцию сравнения, которая возвращала 0, если LHS было меньше, чем RHS, 1, если LHS была больше, чем RHS, и 2, если они были равны.Затем я использовал свойства логического не в c ++ для целых чисел, чтобы получить все мои сравнения в одну строку:

inline bool Matrix::operator<(Matrix &RHS){
  return ! (compare(*this,RHS));
}
inline bool Matrix::operator>(Matrix &RHS){
  return ! (compare((*this),RHS)-1);
}
inline bool Matrix::operator>=(Matrix &RHS){
  return compare((*this),RHS);
}
inline bool Matrix::operator<=(Matrix &RHS){
  return compare((*this),RHS)-1;
}
inline bool Matrix::operator!=(Matrix &RHS){
  return compare((*this),RHS)-2;
}
inline bool Matrix::operator==(Matrix &RHS){
  return !(compare((*this),RHS)-2);
}

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

Согласно предложению, здесь приведен код, если Compare возвращает -1 для меньшего, 0 для равного и 1 для положительного.

inline bool Matrix::operator<(Matrix &RHS){
  return ! (compare(*this,RHS)+1);
}
inline bool Matrix::operator>(Matrix &RHS){
  return ! (compare((*this),RHS)-1);
}
inline bool Matrix::operator>=(Matrix &RHS){
  return compare((*this),RHS)+1;
}
inline bool Matrix::operator<=(Matrix &RHS){
  return compare((*this),RHS)-1;
}
inline bool Matrix::operator!=(Matrix &RHS){
  return compare((*this),RHS);
}
inline bool Matrix::operator==(Matrix &RHS){
  return !(compare((*this),RHS));
}

Я не знаю, что это действительно повышает читаемость, хотя.

Ответы [ 8 ]

22 голосов
/ 17 мая 2010

Да, это слишком умно - я прочитал этот код и должен подумать, почему вы вычитаете два из функции с именем compare. Не заставляй меня думать.

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

Программы должны быть написаны для того, чтобы люди могли читать, и только для машин. (Абельсон и Суссман, Структура и интерпретация компьютерных программ)

8 голосов
/ 17 мая 2010

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

В любом случае, для сравнения, все, что вам когда-либо понадобится, - это < и либо ==, либо !=, остальное канонично, и я пишу это в основном по мышечной памяти. Кроме того, бинарные операторы, относящиеся к своим операндам одинаково (они оставляют их в покое), должны быть реализованы как не являющиеся членами IMO. Учитывая это, плюс с помощью функции сравнения вменяемых (-1, 0, +1) и добавляя необходимые const, я прихожу к этому:

// doing real work
inline bool operator<(const Matrix& l, const Matrix &r)
{
  return -1 == compare(l,r);
}
inline bool operator==(const Matrix& l, const Matrix &r)
{
  return 0 == compare(l,r);
}
// canonical
inline bool operator> (const Matrix& l, const Matrix &r) {return r < l;}
inline bool operator>=(const Matrix& l, const Matrix &r) {return !(l < r);}
inline bool operator<=(const Matrix& l, const Matrix &r) {return !(r < l);}
inline bool operator!=(const Matrix& l, const Matrix &r) {return !(l == r);}

Сравнения могут быть не такими умными, как ваши, но каждый, кто когда-либо видел strcmp(), сразу же знает, что они делают. Обратите внимание, что я даже добавил 0 != compare(...), что совершенно не нужно - для компилятора. Для людей IMO это делает более ясным, что происходит, чем неявное приведение к bool. Кроме того, он подчеркивает симметрию реализации operator<.

8 голосов
/ 17 мая 2010

Да, это слишком сложно.

compare должен возвращать 0 для равных значений, положительный, если this больше, и отрицательный, если this меньше.

Это было бы намного проще и было бы даже производительности.

Если бы я дал этот код для проверки, я бы отметил это как нечто, что должно быть исправлено.

5 голосов
/ 17 мая 2010

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

inline bool Matrix::operator < (const Matrix &RHS){
  return compare(*this, RHS) < 0;
}
inline bool Matrix::operator > (const Matrix &RHS){
  return compare(*this, RHS) > 0;
}
inline bool Matrix::operator >= (const Matrix &RHS){
  return compare(*this, RHS) >= 0;
}
inline bool Matrix::operator <= (const Matrix &RHS){
  return compare(*this, RHS) <= 0;
}
inline bool Matrix::operator != (const Matrix &RHS){
  return compare(*this, RHS) != 0;
}
inline bool Matrix::operator == (const Matrix &RHS){
  return compare(*this, RHS) == 0;
}
3 голосов
/ 17 мая 2010

Если вы беспокоитесь о том, является ли что-то слишком умным ... это, вероятно,: -)

2 голосов
/ 17 мая 2010

Как упоминалось ранее, я думаю, что стандартным способом было бы вернуть <0, когда LHS <RHS,> 0 для LHS> RHS и 0 для равенства.

Но можно мне задать другой вопрос? Зачем вообще здесь использовать перегрузку операторов? Идея, лежащая в основе перегрузки операторов, заключается в (или должно быть) возможности использовать объекты интуитивно понятным способом. Но, насколько мне известно, не существует стандартного определения для сравнения матриц, кроме (для) равенства. По крайней мере, я не знаю ни одного. Так о чем я буду думать, когда читаю M1

Просто позвольте мне угадать: operator <() и operator> () были просто добавлены для полноты, но на самом деле никогда не будут использоваться в реальном мире кода - верно? Если так, не применяйте их.

2 голосов
/ 17 мая 2010

Clever

предупреждение: см. Комментарии

Я думаю, что != неэффективно вызывать compare (который проверяет все значения обеих матриц, чтобы выяснить, какая из них больше), потому что если m1[0][0]!=m2[0][0], то != уже может вернуть false , Поэтому я думаю, что это хорошая идея, чтобы упростить написание этих операторов, и если производительность не имеет значения вообще, ее можно считать умной. Но если производительность имеет значение , это не умно.

Сейф

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

1 голос
/ 17 мая 2010

Я дам вам свой путь:

#include <boost/operators.hpp>

class Matrix: boost::equality_comparable<Matrix
            , boost::less_than_comparable<Matrix
              > >
{
}; // class Matrix

bool operator==(const Matrix&, const Matrix&);

bool operator<(const Matrix&, const Matrix&);

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

// boost::equality_comparable: automatically generate != from ==
// boost::less_than_comparable: automatically generate >, <=, >= from <
// search for Boost.Operators on the web to get more information

Последнее слово: я не знаю о конкретном приложении, которое вы пишете, но используя матрицы, я всегда считал хорошей идеей иметь базовый класс Matrix и некоторый подкласс TMatrix (шаблон, как указывает) с размерами, известными во время компиляции. Затем вы можете указать операторов на TMatrix, которые могут работать только с матрицами схожих измерений, поскольку все остальное является ересью, и, следовательно, иметь диагностику во время компиляции.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...