Избавиться от лишнего if

176

11 сентября 2019, 18:40

Есть классы Deck(колода) и Card(карточка). Задача при добавлении карты в колоду обновить описание колоды. Проблема в том, что при обновлении базы данных, возникают две проверки if на тип карточки (является ли карточка новой или карточкой для повторения)

struct Card
{
    bool is_new();
    bool is_review();
};

struct Deck
{
    int new_cards()    { return m_newCards; }
    int review_cards() { return m_reviewCards; }
    void add_card(Card card)
    {
        m_cards.push_back(card);
        if (card.is_new) { m_newCards++ }
        else if (card.is_review()) { m_reviewCards++ }
        m_totalCards++;
    }
private:
    std::vector<Card> m_cards;
    int m_newCards;
    int m_reviewCards;
    int m_totalCards;
};
struct Mapper
{
    void update_deck(Deck& deck, Card card)
    {
        int newCards {deck.new_cards()};
        int reviewCards {deck.review_cards()};
        int totalCards {deck.total_cards() + 1};
        if (card.is_new()) { newCards++; }
        else if (card.is_review()) { reviewCards++; }
        update(deck, newCards, totalCards, reviewCards);
        deck.add_card(card);
    }
}

Шаблон State вроде не подходит, карточка внутри себя поведение не меняет.

Создать классы NewCard и ReviewCard и перегрузить метода add_card(NewCard), add_card(RepeatCard), но тогда придется держать карточки в разных контейнерах

Можно еще убрать проверку внутри метода add_card(card) и реализовать методы set_new_cards(int), set_repeat_cards(int) у класса Deck

Как лучше поступить?

Answer 1

Я бы сделал следующим образом: класс Card объявляет в себе enum State, который означает состояние карты (новая, просмотренная, перевернутая и т.п. в рамках полета фантазии). Методы, делающие что-то с картой, изменяют её внутренне состояние.

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

Пример кода:

#include <iostream>
#include <vector>
#include <algorithm>
#include <chrono>
using namespace std;
/**
 *  Класс карты.
 */
class Card
{
public:
    /// имеет два состояния: НОВАЯ и ПРОСМОТРЕННАЯ карта
    enum State {CARD_NEW, CARD_REVIEW};
    /// при создании карты устанавливаем ее состояние, как НОВАЯ
    Card() :
        state_(State::CARD_NEW)
    {
    }
    /// при вытаскивании с колоды меняем состояние карты на ПРОСМОТРЕННУЮ
    void showCard()
    {
       state_ = State::CARD_REVIEW;
    }
    State state() const { return state_; }
private:
    State state_;
};
/**
 * Класс колоды.
 */
class Deck
{
public:
    void addCard(const Card& card)
    {
        deck_.push_back(card);
    }
    int total() const
    {
        return deck_.size();
    }
    /**
     * Вышеописанные "вычисляемые" свойства
     */ 
    int newCardsCount() const
    {
        return getCardsCount(Card::State::CARD_NEW);
    }
    int reviewCardsCount() const
    {
        return getCardsCount(Card::State::CARD_REVIEW);
    }
    /**
     * В это место будет отсылочка по тексту ниже.
     */
private:
    int getCardsCount(Card::State state) const
    {
        /// используя лямба-выражение, записываем всё минималистично и красиво
        return count_if(deck_.begin(), deck_.end(), 
                        [state](const Card &card) { return card.state() == state; });
    }
    vector<Card> deck_;
};
int main()
{
    const int count = 100000;
    Card cards[count ];
    Deck deck;
    auto start = chrono::high_resolution_clock::now();
    /// делаем каждую третью карту из десяти созданных показанной   
    for (int i = 0; i < count; i += 3)
    {
        cards[i].showCard();
    }
    /// добавляем все карты в колоду
    for (auto card: cards)
    {
        deck.addCard(card);
    }
    int newCards = deck.newCardsCount();
    int reviewCards = deck.reviewCardsCount();
    auto end = chrono::high_resolution_clock::now();
    int microseconds = chrono::duration_cast<chrono::microseconds>(end - start).count();
    cout << "New cards count: " << newCards << endl
         << "Review cards count: " << reviewCards << endl
         << "execution time: " << microseconds << " microseconds" << endl;
    return 0;
}

Вывод на экран:

New cards count: 66666
Review cards count: 33334
execution time: 1016

В чем же преимущества данного подхода? Они следующие:

Нет проблем со счетчиками. Каждый раз в меняя состояние одной карты в колоде, придется изменить значения как минимум двух счетчиков (выстрелы себе в ногу никто не отменял, а все потому что забыл уменьшить или увеличить какой-то счетчик... а если этот счетчик еще используется где-то для прохода по массиву, то привет Undefined Behavior). А если состояний много...
Простота добавления нового свойства. При появлении необходимости обрабатывать какое-то новое свойство, то добавляем еще одно значение в enum State {CARD_NEW, CARD_REVIEW, CARD_SOME_NEW_STATE}; и следующий метод в место, к которому была обещанная отсылка в коде выше (всего 3 строчки, в случае с if или switch думаю вышло бы больше):

int getSomeNewStateCount() const
{
    return getCardsCount(Card::State::CARD_SOME_NEW_STATE);
}

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

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