Как избежать if/else if chain при классификации заголовка в 8 направлениях?

Ответ 1

#include <iostream>

enum Direction { UP, UP_RIGHT, RIGHT, DOWN_RIGHT, DOWN, DOWN_LEFT, LEFT, UP_LEFT };

Direction GetDirectionForAngle(int angle)
{
    const Direction slices[] = { RIGHT, UP_RIGHT, UP, UP, UP_LEFT, LEFT, LEFT, DOWN_LEFT, DOWN, DOWN, DOWN_RIGHT, RIGHT };
    return slices[(((angle % 360) + 360) % 360) / 30];
}

int main()
{
    // This is just a test case that covers all the possible directions
    for (int i = 15; i < 360; i += 30)
        std::cout << GetDirectionForAngle(i) << ' ';

    return 0;
}

Вот как я это сделаю. (Согласно моему предыдущему комментарию).

Ответ 2

Вы можете использовать map::lower_bound и сохранить верхнюю границу каждого угла на карте.

Ниже приведен рабочий пример:

#include <cassert>
#include <map>

enum Direction
{
    RIGHT,
    UP_RIGHT,
    UP,
    UP_LEFT,
    LEFT,
    DOWN_LEFT,
    DOWN,
    DOWN_RIGHT
};

using AngleDirMap = std::map<int, Direction>;

AngleDirMap map = {
    { 30, RIGHT },
    { 60, UP_RIGHT },
    { 120, UP },
    { 150, UP_LEFT },
    { 210, LEFT },
    { 240, DOWN_LEFT },
    { 300, DOWN },
    { 330, DOWN_RIGHT },
    { 360, RIGHT }
};

Direction direction(int angle)
{
    assert(angle >= 0 && angle <= 360);

    auto it = map.lower_bound(angle);
    return it->second;
}

int main()
{
    Direction d;

    d = direction(45);
    assert(d == UP_RIGHT);

    d = direction(30);
    assert(d == RIGHT);

    d = direction(360);
    assert(d == RIGHT);

    return 0;
}

Ответ 3

Создайте массив, каждый элемент которого связан с блоком 30 градусов:

Car::EDirection dirlist[] = { 
    Car::EDirection::RIGHT, 
    Car::EDirection::UP_RIGHT, 
    Car::EDirection::UP, 
    Car::EDirection::UP, 
    Car::EDirection::UP_LEFT, 
    Car::EDirection::LEFT, 
    Car::EDirection::LEFT, 
    Car::EDirection::DOWN_LEFT,
    Car::EDirection::DOWN, 
    Car::EDirection::DOWN, 
    Car::EDirection::DOWN_RIGHT, 
    Car::EDirection::RIGHT
};

Затем вы можете индексировать массив с помощью угла /30:

this->_car.edir = dirlist[(this->_car.getAbsoluteAngle() % 360) / 30];

Никаких сравнений или разветвлений не требуется.

Результат, однако, немного от оригинала. Значения на границах, то есть 30, 60, 120 и т.д., Помещаются в следующую категорию. Например, в исходном коде допустимые значения для UP_RIGHT равны 31-60. Приведенный выше код присваивает от 30 до 59 к UP_RIGHT.

Мы можем обойти это, вычитая 1 из угла:

this->_car.edir = dirlist[((this->_car.getAbsoluteAngle() - 1) % 360) / 30];

Теперь это дает нам RIGHT для 30, UP_RIGHT для 60 и т.д.

В случае 0 выражение становится (-1 % 360) / 30. Это верно, потому что -1 % 360 == -1 и -1 / 30 == 0, поэтому мы все равно получаем индекс 0.

Раздел 5.6 стандарт С++ подтверждает это поведение:

4 Двоичный оператор / дает частное, а двоичный оператор % дает остаток от деления первого выражение вторым. Если второй операнд / или % равен нулю поведение undefined. Для интегральных операндов оператор /дает алгебраическое отношение с любой дробной частью, отброшенной. если фактор a/b представляется в типе результата, (a/b)*b + a%b равно a.

EDIT:

Было поднято много вопросов относительно удобочитаемости и ремонтопригодности такой конструкции. Ответ, данный motoDrizzt, является хорошим примером упрощения оригинальной конструкции, которая более удобна в обслуживании и не совсем "уродлива".

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

int angle = ((this->_car.getAbsoluteAngle() % 360) + 360) % 360;

this->_car.edir = (angle <= 30)  ?  Car::EDirection::RIGHT :
                  (angle <= 60)  ?  Car::EDirection::UP_RIGHT :
                  (angle <= 120) ?  Car::EDirection::UP :
                  (angle <= 150) ?  Car::EDirection::UP_LEFT :
                  (angle <= 210) ?  Car::EDirection::LEFT : 
                  (angle <= 240) ?  Car::EDirection::DOWN_LEFT :
                  (angle <= 300) ?  Car::EDirection::DOWN:  
                  (angle <= 330) ?  Car::EDirection::DOWN_RIGHT :
                                    Car::EDirection::RIGHT;

Ответ 4

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

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

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

int angle = this->_car.getAbsoluteAngle();

if (angle <= 30 || angle >= 330)
  return Car::EDirection::RIGHT;
else if (angle <= 60)
  return Car::EDirection::UP_RIGHT;
else if (angle <= 120)
  return Car::EDirection::UP;
else if (angle <= 150)
  return Car::EDirection::UP_LEFT;
else if (angle <= 210)
  return Car::EDirection::LEFT;
else if (angle <= 240)
  return Car::EDirection::DOWN_LEFT;
else if (angle <= 300)
  return Car::EDirection::DOWN;
else if (angle <= 330)
  return Car::EDirection::DOWN_RIGHT;

Поместите это в метод, назначьте возвращаемое значение объекту, сверните метод и забудьте о нем на оставшуюся вечность.

P.S. есть еще одна ошибка над порогом 330, но я не знаю, как вы хотите ее обработать, поэтому я не исправил ее вообще.

Позднее обновление

В соответствии с комментарием вы можете даже избавиться от else, если вообще:

int angle = this->_car.getAbsoluteAngle();

if (angle <= 30 || angle >= 330)
  return Car::EDirection::RIGHT;

if (angle <= 60)
  return Car::EDirection::UP_RIGHT;

if (angle <= 120)
  return Car::EDirection::UP;

if (angle <= 150)
  return Car::EDirection::UP_LEFT;

if (angle <= 210)
  return Car::EDirection::LEFT;

if (angle <= 240)
  return Car::EDirection::DOWN_LEFT;

if (angle <= 300)
  return Car::EDirection::DOWN;

if (angle <= 330)
  return Car::EDirection::DOWN_RIGHT;

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

Ответ 5

В псевдокоде:

angle = (angle + 30) %360; // Offset by 30. 

Итак, мы имеем 0-60, 60-90, 90-150,... как категории. В каждом квадранте с 90 градусами одна часть имеет 60, одна часть имеет 30. Итак, теперь:

i = angle / 90; // Figure out the quadrant. Could be 0, 1, 2, 3 

j = (angle - 90 * i) >= 60? 1: 0; // In the quardrant is it perfect (eg: RIGHT) or imperfect (eg: UP_RIGHT)?

index = i * 2 + j;

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

Ответ 6

switch (this->_car.getAbsoluteAngle() / 30) // integer division
{
    case 0:
    case 11: this->_car.edir = Car::EDirection::RIGHT; break;
    case 1: this->_car.edir = Car::EDirection::UP_RIGHT; break;
    ...
    case 10: this->_car.edir = Car::EDirection::DOWN_RIGHT; break;
}

Ответ 7

Игнорируя ваш первый if, который представляет собой бит специального случая, остальные все следуют одному и тому же шаблону: min, max и direction; псевдокод:

if (angle > min && angle <= max)
  _car.edir = direction;

Создание этого реального С++ может выглядеть так:

enum class EDirection {  NONE,
   RIGHT, UP_RIGHT, UP, UP_LEFT, LEFT, DOWN_LEFT, DOWN, DOWN_RIGHT };

struct AngleRange
{
    int min, max;
    EDirection direction;
};

Теперь вместо того, чтобы писать кучу if s, просто перейдите по своим различным возможностям:

EDirection direction_from_angle(int angle, const std::vector<AngleRange>& angleRanges)
{
    for (auto&& angleRange : angleRanges)
    {
        if ((angle > angleRange.min) && (angle <= angleRange.max))
            return angleRange.direction;
    }

    return EDirection::NONE;
}

(throw исключение вместо return ing NONE - это еще один вариант).

Которое вы бы назвали:

_car.edir = direction_from_angle(_car.getAbsoluteAngle(), {
    {30, 60, EDirection::UP_RIGHT},
    {60, 120, EDirection::UP},
    // ... etc.
});

Этот метод известен как управляемое данными программирование. Помимо избавления от кучи if s, это позволит вам легко добавлять дополнительные направления (например, NNW) или уменьшать число (слева, справа, вверх, вниз) без повторного использования другого кода.

(Обращение с первым специальным случаем остается как "упражнение для читателя".:-))

Ответ 8

Хотя предлагаемые варианты, основанные на таблице поиска для angle / 30, вероятно, предпочтительнее, вот альтернатива, которая использует жестко закодированный двоичный поиск, чтобы минимизировать количество сравнений.

static Car::EDirection directionFromAngle( int angle )
{
    if( angle <= 210 )
    {
        if( angle > 120 )
            return angle > 150 ? Car::EDirection::LEFT
                               : Car::EDirection::UP_LEFT;
        if( angle > 30 )
            return angle > 60 ? Car::EDirection::UP
                              : Car::EDirection::UP_RIGHT;
    }
    else // > 210
    {
        if( angle <= 300 )
            return angle > 240 ? Car::EDirection::DOWN
                               : Car::EDirection::DOWN_LEFT;
        if( angle <= 330 )
            return Car::EDirection::DOWN_RIGHT;
    }
    return Car::EDirection::RIGHT; // <= 30 || > 330
}

Ответ 9

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

Прежде всего предположим, что мы используем @Geek Enum

Enum EDirection { RIGHT =0, UP_RIGHT, UP, UP_LEFT, LEFT, DOWN_LEFT,DOWN, DOWN_RIGHT}

Теперь мы можем вычислить перечисление с использованием целочисленной математики (без необходимости в массивах).

EDirection angle2dir(int angle) {
    int d = ( ((angle%360)+360)%360-1)/30;
    d-=d/3; //some directions cover a 60 degree arc
    d%=8;
    //printf ("assert(angle2dir(%3d)==%-10s);\n",angle,dir2str[d]);
    return (EDirection) d;
}

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

assert(angle2dir(  0)==RIGHT     ); assert(angle2dir( 30)==RIGHT     );
assert(angle2dir( 31)==UP_RIGHT  ); assert(angle2dir( 60)==UP_RIGHT  );
assert(angle2dir( 61)==UP        ); assert(angle2dir(120)==UP        );
assert(angle2dir(121)==UP_LEFT   ); assert(angle2dir(150)==UP_LEFT   );
assert(angle2dir(151)==LEFT      ); assert(angle2dir(210)==LEFT      );
assert(angle2dir(211)==DOWN_LEFT ); assert(angle2dir(240)==DOWN_LEFT );
assert(angle2dir(241)==DOWN      ); assert(angle2dir(300)==DOWN      );
assert(angle2dir(301)==DOWN_RIGHT); assert(angle2dir(330)==DOWN_RIGHT);
assert(angle2dir(331)==RIGHT     ); assert(angle2dir(360)==RIGHT     );

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

Ответ 10

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

enum EDirection {
    RIGHT =  0x01,
    LEFT  =  0x02,
    UP    =  0x04,
    DOWN  =  0x08,
    DOWN_RIGHT = DOWN | RIGHT,
    DOWN_LEFT = DOWN | LEFT,
    UP_RIGHT = UP | RIGHT,
    UP_LEFT = UP | LEFT,

    // just so we be clear, these won't have much use though
    IMPOSSIBLE_H = RIGHT | LEFT, 
    IMPOSSIBLE_V = UP | DOWN
};

проверка (псевдокод), если угол равен абсолюту (от 0 до 360):

int up    = (angle >   30 && angle <  150) * EDirection.UP;
int down  = (angle >  210 && angle <  330) * EDirection.DOWN;
int right = (angle <=  60 || angle >= 330) * EDirection.Right;
int left  = (angle >= 120 && angle <= 240) * EDirection.LEFT;

EDirection direction = (Direction)(up | down | right | left);

switch(direction){
    case RIGHT:
         // do right
         break;
    case UP_RIGHT:
         // be honest
         break;
    case UP:
         // whats up
         break;
    case UP_LEFT:
         // do you even left
         break;
    case LEFT:
         // 5 done 3 to go
         break;
    case DOWN_LEFT:
         // your're driving me to a corner here
         break;
    case DOWN:
         // :(
         break;
    case DOWN_RIGHT:
         // completion
         break;

    // hey, we mustn't let these slide
    case IMPOSSIBLE_H:
    case IMPOSSIBLE_V:
        // treat your impossible case here!
        break;
}