Как улучшить этот код? (слишком много, если) - PullRequest
Новая
       5

Как улучшить этот код? (слишком много, если)

1 голос
/ 08 августа 2009

Я хочу напечатать границу квадрата ... Он может печатать только одну сторону или несколько сторон квадрата, поэтому я написал этот метод 

printBorder(N, E, S, W) {
  if (N) {
     square.printBorder(0,0,0,10);
  }
  if (E) {
     square.printBorder(0,10,10,10);
  }
  if (S) {
     square.printBorder(10,0,10,10);
  }
  if (W) {
     square.printBorder(0,0,10,0);
  }
}

Это может хорошо работать, но я думаю, что это не так элегантно, слишком много, если, и все утверждения более или менее одинаковы. Я думаю, что должен быть способ упростить этот код, какие-либо предложения?

Ответы [ 6 ]

5 голосов
/ 08 августа 2009

Мне было бы все равно, если. Я бы просто сделал его более читабельным: 

printBorder(N, E, S, W){
  if(N) square.printBorder( 0,  0,  0, 10);
  if(E) square.printBorder( 0, 10, 10, 10);
  if(S) square.printBorder(10,  0, 10, 10);
  if(W) square.printBorder( 0,  0, 10,  0);
}
5 голосов
/ 08 августа 2009

Один из способов упростить это ... совершать звонки, даже если они вам не нужны, но обусловить реализацию: 

printBorder(N, E, S, W){
  square.printBorder(n, 0,0,0,10);
  square.printBorder(e, 0,10,10,10);
  square.printBorder(s, 10,0,10,10);
  square.printBorder(w, 0,0,10,0);
}

Затем в Square (или как угодно): 

printBorder(condition, top, left, bottom, right) {
  if (!condition) {
    return;
  }
  printBorder(top, left, bottom, right);
}

Аналогичной альтернативой будет сохранение условного printBorder с исходной функцией: 

printBorder(N, E, S, W){
  printBorder(n, 0,0,0,10);
  printBorder(e, 0,10,10,10);
  printBorder(s, 10,0,10,10);
  printBorder(w, 0,0,10,0);
}

printBorder(condition, top, left, bottom, right) {
  if (!condition) {
    return;
  }
  square.printBorder(top, left, bottom, right);
}
3 голосов
/ 08 августа 2009

Как насчет: 

square.printBorder(N|E|W?0:10, N|S|W?0:10, N?0:10, N|E|S?10:0);
3 голосов
/ 08 августа 2009

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

Если вы действительно хотите что-то сделать, посмотрите, если вы не можете переместить параметр Border в квадрат. Вы можете переместить заполнение границ (10 в вашем примере в квадрат), возможно, также указать, какая граница должна быть показана, а затем просто вызвать square.printBorders (). Это во многом зависит от контекста, в котором вы используете это.

3 голосов
/ 08 августа 2009

Лично мне очень нравятся бинарные сравнения. 

const uint NORTH = 1;
const uint SOUTH = 2;
const uint EAST = 4;
const uint WEST = 8;

// ... some code ...
printBorder(NORTH + EAST);
// ... some other code ...

printBorder(uint Sides)
{
   if((NORTH & Sides) > 0) square.printBorder(0, 0, 0, 10);
   if((SOUTH & Sides) > 0) square.printBorder(0, 10, 10, 10);
   if((EAST & Sides) > 0) square.printBorder(10, 0, 10, 10);
   if((WEST & Sides) > 0) square.printBorder(0, 0, 10, 0);
}

Некоторые могут сказать, что это делает код внутри функции менее читабельным. Тем не менее, я думаю, что есть только одно появление этой функции, тогда как вы будете вызывать эту функцию повсюду. Если вы просматриваете какой-то код, который не просматривали некоторое время, который будет более читабельным? 

printBorder(true, false, true, true);

или 

printBorder(NORTH + SOUTH + EAST);

Только мое мнение. :)

1 голос
/ 08 августа 2009

вы не указали какой язык программирования.

если бы это была java, перечисления могут обеспечить хороший читаемый синтаксис, безопасность типов, а также воспользоваться эффективными возможностями поиска битов в реализации EnumSet.

в качестве альтернативы вы также можете предоставить сигнатуру метода varargs, но тогда вы не можете быть уверены, что ваш метод будет вызываться с помощью printBorder (N, N), что в действительности не имеет смысла. используя интерфейс EnumSet у вас есть эта гарантия. 

  public class PrintBorder {

    //this is your method without the if's
    public static void printBorder(EnumSet<Sides> sides) {
        for (Sides side : sides) {
            side.print(square);
        }
    }

    //use it like this
    public static void main(String[] args) {
        printBorder(EnumSet.of(N, E)); //static import here
    }

    //declare an enum for the sides.
    public enum Sides {
        N(0, 0, 0, 10),
        E(0, 10, 10, 10),
        S(10, 0, 10, 10),
        W(0, 0, 10, 0);

        private final int x1;
        private final int y1;
        private final int x2;
        private final int y2;

        Sides(int x1, int y1, int x2, int y2) {
            this.x1 = x1;
            this.y1 = y1;
            this.x2 = x2;
            this.y2 = y2;
        }

        //this method could as well be in the Square class, would be cleaner
        public void print(Square s) {
            s.printBorder(x1, y1, x2, y2);
        }

    }

    //boilerplate here
    private static final Square square = new Square();

    private static class Square {
        public void printBorder(int x1, int y1, int x2, int y2) {
            //do something..
        }
    }
}
...