2013-05-21 3 views
24

Приведенная ниже программа функционирует по мере необходимости, но как уменьшить количество операторов if. Мне сказали, что если ваша функция содержит 2 или более оператора if, то вы делаете это неправильно. Какие-либо предложения? Я пробовал использовать операторы switch, но это не сработало, потому что случай не может быть логическим.Как уменьшить, если операторы

for(int i = 1; i < 100; i++) 
     { 
     if(i % 10 == 3) 
     { 
      System.out.println("Fizz" + "(" + i + ") 3%10"); 
     } 

     if(i/10 == 3) 
     { 
      System.out.println("Fizz" + "(" + i + ") 3/10"); 
     } 


     if(i % 10 == 5) 
     { 
      System.out.println("Buzz" + "(" + i + ") 5%10"); 
     } 

     if(i/10 == 5) 
     { 
      System.out.println("Fizz" + "(" + i + ") 5/10"); 
     } 

     if(i/10 == 7) 
     { 
      System.out.println("Fizz" + "(" + i + ") 7/10"); 
     } 

     if(i%10 == 7) 
     { 
      System.out.println("Woof" + "(" + i + ") 7%10"); 
     } 

     if(i % 3 == 0) 
     { 
      System.out.println("Fizz" + "(" + i + ") 3%==0"); 
     } 

     if(i % 5 == 0) 
     { 
      System.out.println("Buzz" + "(" + i + ")5%==0"); 
     } 

     if(i % 7 == 0) 
     { 
      System.out.println("Woof" + "(" + i + ")7%==0");  
     } 

     if((i % 7 !=0) && (i % 3 !=0) && (i % 5 !=0) 
       && (i % 10 !=3) && (i % 10 !=5) && (i%10 !=7)) 
      System.out.println(i); 
    } 
+0

Это все о том, как вы хотите разработать свое приложение. – OsakaHQ

+4

Является ли это домашним заданием или тестовым вопросом? – JayDM

+0

Его тип вопроса интервью. – Calgar99

ответ

49

Как о создании метода для случаев:

public void printIfMod(int value, int mod){ 
     if (value % 10 == mod) 
      System.out.println(...); 
} 

public void printIfDiv(int value, int div){ 
     if (value/10 == div) 
      System.out.println(...); 
} 

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

public void printIf(int value, int div){ 
     printIfMod(value, div); 
     printIfDiv(value, div); 
} 

for(int i = 1; i < 100; i++) { 
     printIf(i, 3); 
     printIf(i, 5); 
     .... 
} 

В приведенном выше коде число ifs является менее важной проблемой для меня, чем количество повторяющегося кода.

+0

Вы также можете перебирать вложенные значения для значений в 1 , 3, 5, 7 и т. Д., Чтобы избежать повторения этих –

+3

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

+2

Вам нужно добавить «звук», как в ответе AmitG. 'Public void printIfXyz (string sound, int value, int numberator) {...}'. Код в примере не является чисто 3 = fizz/5 = buzz/7 = woof. (Это преднамеренное или преднамеренное неизвестно). – WernerCD

26

Вот небольшое улучшение с помощью двух переключателей заявления

switch(i/10){ 
    case 3: // do something 
    break; 
    case 5: // do something else 
    break; 
    case 7: // do something else 
    break; 
} 

switch(i % 10){ 
    case 3: // do something 
    break; 
    case 5: // do something else 
    break; 
    case 7: // do something else 
    break; 
} 

К сожалению, вам потребуется один оператор коммутатора каждого делителя.

В качестве альтернативы, вы можете принять ООП и придумать абстракции, как это: Использование

public abstract class Processor { 
    private final int divisor; 
    private final int result; 
    private final boolean useDiv; // if true, use /, else use % 

    public Processor(int divisor, int result, boolean useDiv) { 
     this.divisor = divisor; 
     this.result = result; 
     this.useDiv = useDiv; 
    } 
    public final void process(int i){ 
     if (
      (useDiv && i/divisor == result) 
      || (!useDiv && i % divisor == result) 
      ){ 
       doProcess(i); 
      } 
    } 

    protected abstract void doProcess(int i); 
} 

Пример:

public static void main(String[] args) { 
    List<Processor> processors = new ArrayList<>(); 
    processors.add(new Processor(10, 3, false) { 
     @Override 
     protected void doProcess(int i) { 
      System.out.println("Fizz" + "(" + i + ") 3%10"); 
     } 
    }); 
    // add more processors here 
    for(int i = 1; i < 100; i++){ 
     for (Processor processor : processors) { 
      processor.process(i); 
     } 
    } 

} 
12

Вообще говоря, это правда, что код, который имеет много if заявлений выглядит подозрительно. Подозрительный не обязательно означает неправильное. Если в задаче проблемы есть непересекающиеся условия для проверки (т. Е. Вы не можете их сгруппировать), тогда вы должны делать их независимо, как вы делаете.

В вашем случае вам нужно проверить делимость, не имея возможности вывести один из другого (т. Е. Если x делится на 7, это не значит, что оно также делится на 5 и т. Д.) , Все номера, которые вы используете, были специально выбраны просто так, поэтому вы вникаете в это.

Если, например, они сказали, проверьте делимость на 2, 3 и 6. Затем вы можете сначала проверить 6, потому что тогда вы также можете указать делимость на 2 и 3. Или наоборот, проверьте на 2 и 3 и подразумевают, что он также делится на 6. Если все числа являются первичными, то вы просто не можете подразумевать. Таким образом, вам нужен код для проверки по отдельности.

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

Мои два цента на это ...

1

Вы можете создать несколько переключатель:

switch (i/10) { 
    case 3: 
     System.out.println("Fizz" + "(" + i + ") 3/10"); 
     break; 

    case 5: 
     System.out.println("Fizz" + "(" + i + ") 5/10"); 
     break; 

    case 7: 
     System.out.println("Fizz" + "(" + i + ") 7/10"); 
     break; 
    default: 
     break; 
} 

switch (i%10) { 
    case 3: 
     System.out.println("Fizz" + "(" + i + ") 3%10"); 
     break; 
    case 5: 
     System.out.println("Buzz" + "(" + i + ") 5%10"); 
     break; 
    case 7: 
     System.out.println("Woof" + "(" + i + ") 7%10"); 
     break; 
    default: 
     break; 
} 

Другой случай все равно придется использовать, если заявление.
Oracle добавил оператор switch, который использовал String в Java 7. Возможно, инструкция boolean switch будет представлена ​​позже.

+0

нет точки для логического 'switch', это просто' if'/'else'. – Kevin

+0

С помощью boolean switch я имею в виду: 'switch (i)' [...] 'case (i% 10 == 3): commands' [...]' case (i/10 == 5): инструкции'. Это не цель коммутатора, но это сэкономит много времени. – DeadlyJesus

+1

Ах да. Проблема с этим заключается в том, что если 'i% 10 == 3' и' i/10 == 5' (т.е. 53), он хочет, чтобы оба были напечатаны, но этот 'switch' ударил бы только один, предположительно первый. – Kevin

7

Я начал писать ответ с кодом, но многие, многие избили меня.Единственное, что я хотел бы сказать, о котором еще не упоминалось, это то, что эта конкретная метрика кода, о которой вы говорите, называется cyclomatic complexity и не является ужасно плохим.

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

о, и я бы заменить этот последний раздел:

if((i % 7 !=0) && (i % 3 !=0) && (i % 5 !=0) 
      && (i % 10 !=3) && (i % 10 !=5) && (i%10 !=7)) 
     System.out.println(i); 

используя логический флаг, такой как replaced = true всякий раз, когда какой-либо из операторов замен называется, то приведенное выше утверждение коллапсирует :

if (!replaced) 
     System.out.println(i); 
+1

+1 для ссылки на предыдущую работу по теме условной сложности. – LarsH

0
public class Test 
{ 

    public static void main(String[] args) 
    { 

     final int THREE = 3; 
     final int FIVE = 5; 
     final int SEVEN=7; 
     final int ZERO = 0; 

     for (int i = 1; i < 100; i++) 
     { 
      modOperation("Fizz", i, THREE); 

      divideOperation("Fizz", i, THREE); 


      modOperation("Fizz", i, FIVE); 

      divideOperation("Buzz", i, FIVE); 



      modOperation("Woof", i, SEVEN); 

      divideOperation("Fizz", i, SEVEN); 


      modOperation("Fizz", i, ZERO); 

      divideOperation("Fizz", i, ZERO); 
     } 

    } 

    private static void divideOperation(String sound, int i, int j) 
    { 
     if (i/10 == j) // you can add/expand one more parameter for 10 and later on 3 in this example. 
     { 
      System.out.println(sound + "(" + i + ") "+j+"/10"); 
     } 
    } 

    private static void modOperation(String sound, int i, int j) 
    { 
     if (i % 10 == j) 
     { 
      System.out.println(sound + "(" + i + ") "+j+"%10"); 
     } 
    } 
} 

Так что теперь у вас есть меньше if

+1

Совпадает с более высоким рейтингом ответа ... но не уверен, что я понимаю необходимость «ТРИ», «ПЯТЬ», ... Как вы думаете, вам нужно будет в какой-то момент обновить значение этих чисел? – WernerCD

+0

@WernerCD :-) да. Ты прав. Вы можете передать эти значения напрямую. Включая свою мысль выше, мой ответ будет своего рода еще одним возможным решением для ОП. – AmitG

8

Перечисления хорошо подходят здесь. Они позволяют вам инкапсулировать функциональность в одном месте, а не распространять ее во время управления потоком.

public class Test { 
    public enum FizzBuzz { 
    Fizz { 
     @Override 
     String doIt(int n) { 
     return (n % 10) == 3 ? "3%10" 
       : (n/10) == 3 ? "3/10" 
       : (n/10) == 5 ? "5/10" 
       : (n/10) == 7 ? "7/10" 
       : (n % 3) == 0 ? "3%==0" 
       : null; 
     } 

    }, 
    Buzz { 
     @Override 
     String doIt(int n) { 
     return (n % 10) == 5 ? "5%10" 
       : (n % 5) == 0 ? "5%==0" 
       : (n/10) == 3 ? "3/10" 
       : (n/10) == 5 ? "5/10" 
       : (n/10) == 7 ? "7/10" 
       : null; 
     } 

    }, 
    Woof { 
     @Override 
     String doIt(int n) { 
     return (n % 10) == 7 ? "7%10" 
       : (n % 7) == 0 ? "7%==0" 
       : null; 
     } 

    }; 

    // Returns a String if this one is appropriate for this n. 
    abstract String doIt(int n); 

    } 

    public void test() { 
    // Duplicates the posters output. 
    for (int i = 1; i < 100; i++) { 
     boolean doneIt = false; 
     for (FizzBuzz fb : FizzBuzz.values()) { 
     String s = fb.doIt(i); 
     if (s != null) { 
      System.out.println(fb + "(" + i + ") " + s); 
      doneIt = true; 
     } 
     } 
     if (!doneIt) { 
     System.out.println(i); 
     } 
    } 
    // Implements the game. 
    for (int i = 1; i < 100; i++) { 
     boolean doneIt = false; 
     for (FizzBuzz fb : FizzBuzz.values()) { 
     String s = fb.doIt(i); 
     if (s != null) { 
      if (doneIt) { 
      System.out.print("-"); 
      } 
      System.out.print(fb); 
      doneIt = true; 
     } 
     } 
     if (!doneIt) { 
     System.out.print(i); 
     } 
     System.out.println(); 
    } 
    } 

    public static void main(String args[]) { 
    try { 
     new Test().test(); 
    } catch (Throwable t) { 
     t.printStackTrace(System.err); 
    } 
    } 

} 
+2

+1 Хотя мне нравится подход enum, я действительно не могу выдержать вложенные условные операторы. Но это может быть личное предпочтение. Это просто мое эмпирическое правило, чтобы никогда не устанавливать условные операторы. – helpermethod

+0

@helpermethod - я могу понять ваши предпочтения - исходя из C-фона, это кажется вполне естественным. В 'Algol' также были сокращены формы для циклов и' case'. – OldCurmudgeon

5

Ваш код повторяющийся. Refactor это с использованием петель для вашего:

for (int i = 1; i < 100; i++) { 
    boolean found = false; // used to avoid the lengthy test for "nothing found" 
    for (int j = 3; j <= 7; j += 2) { // loop 3, 5, 7 
     if (i % 10 == j) { 
      System.out.println("Fizz" + "(" + i + ") "+j+"%10"); 
      found = true; 
     } 

     if (i/10 == j) { 
      System.out.println("Fizz" + "(" + i + ") "+j+"/10"); 
      found = true; 
     } 

     if (i % j == 0) { 
      System.out.println("Fizz" + "(" + i + ") "+j+"%==0"); 
      found = true; 
     } 
    } 

    if (!found) { 
     System.out.println(i); 
    } 
} 
+2

@ugoren поэтому ... фиксированный. '' Я набрал этот код на моем iPhone '' – Bohemian

+0

Почти там - 'j <= 7'. – ugoren

+0

@ugoren yup .... – Bohemian

7

Я бы сказал, что вы задаете неправильный вопрос. Вопрос, который, я думаю, вам следует задать, это: «Как я могу переписать этот код так, чтобы его легче понять человеком?»

Вероучение «исключить, если утверждения» - общая идея для достижения этого, но в значительной степени зависит от контекста.

Печальный факт заключается в том, что многие из ответов обманывают этот очень простой алгоритм под видом «упрощения». Никогда не вводите объект для устранения нескольких утверждений if. В моей работе большинство кодов поддерживаются людьми, которые гораздо меньше понимают архитектуру, математику и код, чем оригинальный автор, поэтому вводят дополнительные конструкции и сложность для уменьшения кода от 50 физических линий до 30 физических линий, но делает его 4 раз труднее понять, это не победа.

+1

Вы имеете в виду - t подписаться на «Если было сложно программировать, это должно быть трудно понять».? ;-) Пример кода, который здесь находится в вопросе и ответах, не содержит значимых имен в переменной и константах, что сделало бы функцию и назначение немного более очевидными. – Rawheiser

Смежные вопросы