使用 DRY 原则重构逻辑

Question

在使用 C++ 编程时，我偶然发现了这样的源代码：

int enemy = 1; //enemy can be 1 or -1
if (enemy == 1) {
    for (short i = 1; i < 100; i++) {
        if (some_array[i] <= enemy) {
            cout << i << ", ";
            do_sth(i);
        }
    }
} else if (enemy == -1) {
    for (short i = 1; i < 100; i++) {
        if (some_array[i] >= enemy) {
            cout << i << ", ";
            do_sth_else_here(i);
        }
    }
}

毫无疑问，代码做了它应该做的事情，但有悖于DRY并且也很混乱。

有没有办法缩短代码（例如，当enemy为负[=21=时，将大于号换成小于号]) 或重构它以更好地适应 DRY 原则？

Answer 1

我认为我们可以在不损失太多可读性的情况下合理地将函数简化为这样的东西：

第一次尝试

void handle_enemy(int enemy)
{
    assert(enemy == -1 or enemy == 1);
    if (enemy == 1) {
        check_enemy(enemy, std::less_equal<>(), &do_sth);
    } else if (enemy == -1) {
        check_enemy(enemy, std::greater_equal<>(), &do_sth_else_here);
    }
}

其中 check_enemy 已重构为：

template<class Comparer, class Action>
void check_enemy(int enemy, Comparer comp, Action action)
{
    for (short i = 1; i < some_array.size(); i++) {
        if (comp(some_array[i], enemy))
        {
            std::cout << i;
            action(i);
            std::cout << ", ";
        }
    }
    std::cout << std::endl;
}

完整的工作示例：

#include <iostream>
#include <array>
#include <random>
#include <algorithm>
#include <cassert>

void do_sth(int i) { std::cout << '!' ; }
void do_sth_else_here(int i) { std::cout << '?'; }

std::array<int, 100> some_array;

template<class Comparer, class Action>
void check_enemy(int enemy, Comparer comp, Action action)
{
    for (short i = 1; i < some_array.size(); i++) {
        if (comp(some_array[i], enemy))
        {
            std::cout << i;
            action(i);
            std::cout << ", ";
        }
    }
    std::cout << std::endl;
}

void handle_enemy(int enemy)
{
    assert(enemy == -1 or enemy == 1);
    if (enemy == 1) {
        check_enemy(enemy, std::less_equal<>(), &do_sth);
    } else if (enemy == -1) {
        check_enemy(enemy, std::greater_equal<>(), &do_sth_else_here);
    }
}

int main()
{
    std::default_random_engine eng(std::random_device{}());
    std::generate(std::begin(some_array),
                  std::end(some_array),
                  [&eng,
                   dist = std::uniform_int_distribution<int>(-5, +5)]() mutable -> int
    {
        return dist(eng);
    });
    handle_enemy(-1);
    handle_enemy(1);

    return 0;
}

示例输出：

1?, 3?, 4?, 5?, 6?, 7?, 10?, 11?, 13?, 15?, 16?, 17?, 18?, 21?, 22?, 23?, 24?, 25?, 26?, 27?, 28?, 30?, 32?, 33?, 34?, 35?, 39?, 40?, 42?, 43?, 45?, 46?, 48?, 49?, 51?, 52?, 53?, 56?, 57?, 58?, 59?, 60?, 61?, 62?, 63?, 65?, 66?, 67?, 68?, 69?, 71?, 73?, 74?, 75?, 76?, 78?, 79?, 80?, 81?, 83?, 84?, 86?, 87?, 88?, 90?, 91?, 92?, 95?, 96?, 97?, 98?, 99?,
2!, 3!, 4!, 5!, 7!, 8!, 9!, 10!, 12!, 14!, 18!, 19!, 20!, 23!, 27!, 28!, 29!, 30!, 31!, 33!, 34!, 36!, 37!, 38!, 39!, 40!, 41!, 42!, 44!, 47!, 50!, 52!, 53!, 54!, 55!, 58!, 61!, 63!, 64!, 65!, 66!, 67!, 68!, 70!, 72!, 75!, 76!, 77!, 81!, 82!, 83!, 85!, 86!, 89!, 90!, 91!, 93!, 94!, 95!, 99!,

但是我们可以做得更好吗？

好吧，我认为这取决于您希望获得多少 DRY，以及您希望将规则与逻辑分离多少。

这个版本定义了一个actionerclass。重载的调用运算符只会为 enemy 的值编译，这些值具有为其定义的完整规则集。

这可能符合您的喜好。在我看来，它开始使程序脱钩，以至于对作者以外的任何人来说，维护它都可能是个问题。

你可能会说它是 SuperDRY [谢谢，我整个星期都在这里:-)]

#include <iostream>
#include <array>
#include <random>
#include <algorithm>
#include <cassert>

void do_sth(int i) { std::cout << '!' ; }
void do_sth_else_here(int i) { std::cout << '?'; }

std::array<int, 100> some_array;

// a tag which turns an enemy value into a type, allowing us to easily tag-dispatch our rules.
template<int Enemy>
struct enemy_tag {
    static constexpr int value = Enemy;
};

// A function object that contains all the rules for this action
struct enemy_actioner
{
    constexpr enemy_actioner(std::array<int, 100> const& the_array)
    : _the_array(the_array)
    {}

    // Define logic once

    template<int Enemy, class Pred, class Action>
    void logic(Pred pred, Action action) const
    {
        for (int i = 1 ; i < _the_array.size() ; ++i)
        {
            if (pred(_the_array[i], Enemy))
            {
                std::cout << i;
                action(i);
                std::cout << ", ";
            }
        }
    }

    // Define Rules Once:

    static constexpr auto predicate_for(enemy_tag<1>) { return std::less_equal<>(); }
    static constexpr auto predicate_for(enemy_tag<-1>) { return std::greater_equal<>(); }

    static constexpr auto action_for(enemy_tag<1>) { return &do_sth; }
    static constexpr auto action_for(enemy_tag<-1>) { return do_sth_else_here; }

    // glue logic and rules together through the enemy_tag

    template<int Enemy>
    void operator()(enemy_tag<Enemy> enemy) const
    {
        logic<enemy.value>(predicate_for(enemy), action_for(enemy));
        std::cout << std::endl;
    }

private:
    std::array<int, 100> const& _the_array;
};


void handle_enemy(int enemy)
{
    // introduce our now opaque action object
    enemy_actioner action(some_array);

    // now all we need to do is turn the enemy integer into a tag and the actioner takes
    // care of all the rest. Truly DRY.
    switch (enemy)
    {
        case 1: return action(enemy_tag<1>());
        case -1: return action(enemy_tag<-1>());
        default: assert(!"logic error in program");
    }
}

int main()
{
    std::default_random_engine eng(std::random_device{}());
    std::generate(std::begin(some_array),
                  std::end(some_array),
                  [&eng,
                   dist = std::uniform_int_distribution<int>(-5, +5)]() mutable -> int
                  {
                      return dist(eng);
                  });
    handle_enemy(-1);
    handle_enemy(1);

    return 0;
}

使用 DRY 原则重构逻辑

Refactor Logic using DRY principles

c++

compare

dry

第一次尝试

但是我们可以做得更好吗？