c++ - 使用DRY原理重构逻辑

Closed. This question is opinion-based。它当前不接受答案。
                            
                        
                    
                
                            
                                
                
                        
                            
                        
                    
                        
                            想改善这个问题吗？更新问题，以便editing this post用事实和引用来回答。
                        
                        3年前关闭。
                                                                                            
                
        
使用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为负数时，将大于的代码交换为较小的代码）或对其进行重构以更好地适应DRY原理？

最佳答案

我认为我们可以合理地将功能减少到这样的程度，而不会失去太多的可读性：

第一次尝试

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，以及您希望将规则与逻辑脱钩的程度。

该版本定义了actioner类。重载的调用运算符将​​仅编译具有为其定义了完整规则集的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;
}