Рефакторинг связанные условия без объединения - PullRequest
0 голосов
/ 01 декабря 2011

У меня есть пиксель / местоположение, мы будем называть его 4,4 на графике. Я пытаюсь реорганизовать функцию, которая проверяет, соответствуют ли пиксели вокруг нее определенным критериям во всех 8 направлениях (по диагонали, по горизонтали и по вертикали), и, если она соответствует этим критериям, предпринимают общее действие, которое имеет возвращаемое значение.

Например:

int weight=0;
if (CheckWeight(new Point(4,4)) == true)
    weight +=100;
if (CheckWeight(new Point(4,5)) == true)
    weight +=10;
if (CheckWeight(new Point(4,3)) == true)
    weight +=10;
if (CheckWeight(new Point(5,5)) == true)
    weight +=10;
if (CheckWeight(new Point(3,3)) == true)
    weight +=10;
if (CheckWeight(new Point(3,4)) == true)
    weight +=10;
if (CheckWeight(new Point(5,4)) == true)
    weight +=10;
if (CheckWeight(new Point(5,3)) == true)
    weight +=10;
if (CheckWeight(new Point(3,5)) == true)
    weight +=10;

Есть ли хороший способ их реорганизовать, так что если мне придется изменить некоторые вещи здесь, такие как функция, которую я вызываю, чтобы проверить вес, или условие, с которым я проверяю, или приращение веса, что я не дублируете мои усилия 8 раз?

У меня были другие программисты, которых я знаю, которые уже предлагают просто объединить их под одну проверку, что я, очевидно, не могу сделать, потому что она может выполнить 3 из этих проверок и дать мне вес 30 на этот раз и 5 из этих проверок и в следующий раз дадим мне вес 50.

Редактировать: Эта процедура будет выполняться на картах пикселей 1920x1080, то есть несколько миллионов раз; производительность может быть реальной проблемой, связанной с рефакторингом.

Ответы [ 3 ]

3 голосов
/ 01 декабря 2011
int weight = 0;
for (int dx = -1 ; dx <= 1 ; dx++) {
    for (int dy = -1 ; dy <= 1 ; dy++) {
        if (CheckWeight(new Point(4+dx, 4+dy))) {
            weight += (dx==0 && dy == 0) ? 100 : 10;
        }
     }
}

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

// Declare this in your class
static readonly int[] dd = new int{1,-1,-1,0,-1,1,0,1,1};

// Use this code to calculate the weight
int weight = CheckWeight(new Point(4, 4)) ? 100 : 0;
for (int i = 0 ; i != 8 ; i++) {
    if (CheckWeight(new Point(4+dd[i], 4+dd[i+1]))) {
        weight += 10;
    }
}
2 голосов
/ 01 декабря 2011

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

var target = new Point(4, 4);

int weight = 0;
if (CheckWeight(target) == true)
    weight += 100;

var points = GetNearestPointsFrom(target);

foreach (var p in points)
{
    if (CheckWeight(p) == true)
        weight += 10;
}

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


Обновление : Принимая во внимание, что вы ограничены ограничениями производительности и упоминаете, что вам может понадобиться изменить CheckWeight другой функцией, я бы проверил, если обернуть проверки if в методе, который принимает Predicate<Point> и вес значение приращения не сильно повлияет на производительность. Таким образом, изменение метода CheckWeight или приращение выполняется только один раз. Пример кода:

private static void GetWeight(Predicate<Point> predicate, int weightIncrement)
{
    int weight = 0;
    if (predicate(new Point(4, 4)) == true)
        weight += 100;
    if (predicate(new Point(4, 5)) == true)
        weight += weightIncrement;
    // ... Remaining checks ...
}
2 голосов
/ 01 декабря 2011

А как насчет циклов for?

int x = 4;
int y = 4;
int weight = 0; 
for (int dx = -1; dx <= +1; dx++) {
    for (int dy = -1; dy <= +1; dy++) {
        if (CheckWeight(new Point(x+dx, y+dy)))
            weight += 10;
    }
}
...