Какой FizzBuzz лучше, и почему?

Я был в интервью, и парень попросил меня сделать типичный вопрос FizzBuzz о печати номеров 1-100, но для каждого коэффициента 3 печатают Fizz, каждый фактор из 5 печатает Buzz, и каждый фактор обоих печатает FizzBuzz. Я написал следующее:

public static void fizzbuzz(){
    for(int i=1;i<=100;i++){
        if(i%15 == 0){
            System.out.println("FizzBuzz");
        } else if (i%3 == 0) {
            System.out.println("Fizz");
        } else if (i%5 == 0) {
            System.out.println("Buzz");
        } else {
            System.out.println(i);
        }
    }
}

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

public static void fizzbuzz(){
    for(int i=1;i<=100;i++){
        String str = "";
        if(i%3 == 0){
            str = "Fizz";
        }
        if(i%5 == 0){
            str += "Buzz";
        }
        if(str.equals("")){
            str = i;
        }
        System.out.println(str);
    }
}

Затем он спросил меня, что я предпочел бы, если бы мне был вручен этот код, и сказал, что нет неправильного ответа (который я должен был знать, было ложью), но я глупо сказал мне, потому что мне было легче смотреть и что в моем коде происходит что-то другое по коэффициентам в 15, просто просматривая его, тогда как, не процитировав мой код, я бы не знал об этой второй версии.

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

36 голосов | спросил arc4randall 15 PM00000070000005231 2014, 19:36:52

10 ответов


29

Я предпочитаю ваше решение. Мое рассуждение таково:

  • у вас есть константы только при манипулировании строками
  • вы не выполняете конкатенацию строк
  • логика различна для каждого остатка.

Оба варианта могут быть лучше. Отсутствие «пространства передышки» (пробел между ключевыми словами, переменными, значениями и операторами) приводит к трудно читаемому коду. Линии вроде:

for(int i=1;i<=100;i++){

должен быть:

for (int i = 1; i <= 100; i++) {

Тогда ваше решение будет более эффективным в качестве оператора switch:

for (int i = 1; i <= 100; i++) {
    switch (i % 15) {
        case 0:
            System.out.println("FizzBuzz");
            break;
        case 3:
        case 6:
        case 9:
        case 12:
            System.out.println("Fizz");
            break;
        case 5:
        case 10:
            System.out.println("Buzz");
            break;
        default:
            System.out.println(i);
    }
}

Оператор switch будет оптимизирован таким образом, чтобы не было более одного модуля, и нет понятия if/elseif/elsif/else ... только одно условие проверить.

ответил rolfl 15 PM00000070000004631 2014, 19:52:46
21
  

Затем он спросил меня, что я предпочел бы, если бы мне был вручен этот код

Вот что я бы сказал: Это зависит .

rolfl уже добавил некоторые комментарии к вашему решению, поэтому я добавлю несколько комментариев (за и против) о втором решении.

Во-первых, комментарий rolfl в отношении интервала также применим и здесь.

Самый большой про его решение заключается в том, что он более гибкий /расширяемый. Если вы хотите добавить Woof, Ping, Plop , вы можете легко сделать это, не имея напишите много кода.

Однако минусы:

  • Используется String (вместо StringBuilder)), который создает намного больше объектов.
  • Он не компилируется как str = i; является незаконным назначением (строка не может быть установлена ​​в int). Вам нужно будет использовать str = String.valueOf(i); или Integer.toString(i);
  • Гораздо лучше использовать if (str.isEmpty()), а не if (str.equals(""))
  • Вместо того, чтобы установить строку в целое число, если оно пустое, используйте System.out.println(i); или System.out.println(str);, чтобы получить некоторую скорость. (Поскольку вам не нужно создавать строку для целого числа)

Итак, в конце концов, все зависит от того, что вы хотите. Если вы хотите повысить скорость компромисса, то вторая версия лучше (как только вы исправите указанные выше пункты).

Посмотрев только на скорость, ваша оригинальная версия лучше (особенно после предложения rolfl использовать коммутатор, хотя это немного уменьшает ремонтопригодность кода).

Это не все черное и белое, здесь много плюсов и минусов. Я бы поднял эти плюсы и минусы, когда меня задали этот вопрос, а затем спросите интервьюера: «Какова вероятность того, что нам нужно добавить Woof, Ping, Plop позже?» (Примечание: у меня нет но так ответили на собеседование)

ответил Simon Forsberg 15 PM00000080000001231 2014, 20:06:12
18

Это обзор второго - я думаю, что первый из них вполне приемлем, второй - у меня проблемы.

Строки

Там есть строка, которая печатается в конце. ОК. Однако, когда целое число делится на 5, строка изменяется с помощью команды: str += "Buzz";

Это каждый раз строит новый объект String через этот блок, который полностью не подходит для String. Если он должен быть изменчивым, сделайте вместо него StringBuilder.

Я знаю, что люди пойдут ", но это только ударит его 20 раз - это 20 новых объектов, которые заключают сделку?" И ответ «нет», 20 новых объектов не имеют большого значения, но это свидетельствует о том, кто поместит его в цикл, который будет выполняться несколько тысяч раз и задаться вопросом, куда идет память »(я исправил этот код раньше).

Сравнение строк

Оставляя в стороне += для строки String, заключительный оператор if в этом блоке:

    if(str.equals("")){
        str = i;
    }

Мы тестируем строку с строкой с .equals(""). Из-за Интерпретация строк этот "" - это точно тот же объект, что и "" в начале цикла.

Это означает, что тест мог выполняться как:

    if(str == ""){
        str = i;
    }

Теперь я сделаю отказ от ответственности, что, хотя это правильно и будет быстрее, инструменты статического анализа будут жаловаться на это, и люди, смотрящие на него, будут думать о своей ошибке. Вы, вероятно, не должны делать этого таким образом ... И он собирается сделать этот тест в любом случае в первом тесте . equals (Object anObject) в классе String.

У ворча, который у меня есть, есть более одно из них, если String not "", тогда есть еще много тестов, которые нужно выполнить перед этим возвращается к false.

Тест:

    if(str.length() == 0){
        str = i;
    }

или

    if(str.isEmpty()){
        str = i;
    }

- лучший, более лаконичный тест, который точно говорит о том, что он означает, не пройдя весь hoopla кода в .equals().


Кажется, есть некоторая путаница в моей позиции в тестах String.

    if(str == ""){
        str = i;
    }

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

Выполнение .equals("") в Java чрезмерно. Один из проверок статического анализа, который IntelliJ имеет в разделе «Проблемы производительности», - это String.equals("")

  

Вызывается отчет .equals () для сравнения строки с пустой строкой. Обычно более эффективно тестировать String для пустоты, сравнивая его .length() с нулем.

Таким образом, опции должны считаться для этого:

    if(str.length() == 0){
        str = i;
    }

или

    if(str.isEmpty()){
        str = i;
    }

(я бы пошел за isEmpty() поверх length() == 0, хотя в конце дня это то же самое).

Итак, почему это проблема? Давайте посмотрим на код для . Equals (Object anObject) (код немного отличается в 6-b14, чем 7u40-b43 через 8-b132 , хотя он по сути тот же).

1012    public boolean equals(Object anObject) {
1013        if (this == anObject) {
1014            return true;
1015        }
1016        if (anObject instanceof String) {
1017            String anotherString = (String)anObject;
1018            int n = count;
1019            if (n == anotherString.count) {
1020                char v1[] = value;
1021                char v2[] = anotherString.value;
1022                int i = offset;
1023                int j = anotherString.offset;
1024                while (n-- != 0) {
1025                    if (v1[i++] != v2[j++])
1026                        return false;
1027                }
1028                return true;
1029            }
1030        }
1031        return false;
1032    }

Если это объект тот же (это будет верно для случая, когда "" проваливается, потому что ни i%3, ни i% 5, он выйдет прямо на фронт, и мы закончили. Однако, если строка i%5 или "Fizz" или "Buzz", он будет проверять, чтобы убедиться, что экземпляр String затем проверяет счет (закрытое конечное значение, установленное в конструкторе).

"FizzBuzz" для count равен 0, а остальные строки - 4 или 8, и тогда мы перейдем к возврату на 1031 с ложным .

Так зачем же это делать?

Ну, код для "":

isEmpty()

Действительно коротко и точно. Кроткость метода имеет значение. Один из этих 670 public boolean isEmpty() { 671 return count == 0; 672 } вызовов, к которым никто никогда не прикасается (и они не должны знать, что они делают): -XX ( Параметры Java HotSpot VM ). Значение по умолчанию для этого байта составляет 35 байтов.

Если у вас есть небольшой метод, HotSpot будет встроить этот метод. Нет рамки стека, нет поиска метода после того, как HotSpot решит, что он что-то сделает. Его просто smack прямо в коде.

-XX:MaxInlineSize=#

становится тем, что больше похоже на

    if(str.isEmpty()){
        str = i;
    }

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

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

    if(str.count == 0){
        str = i;
    }

Является ли строка пустой? ... и все готово. Цель понятна, и код проще.

Если бы один из них ранее был переключен на StringBuilder для этого, нет вызова if(str.isEmpty()){ , но есть вызов isEmpty(), который возвращает счетчик, который должен использоваться ( источник в AbstractStringBuilder 6-14 ).


Как насчет байтового кода для StringBuilder.append vs length() со строкой? += гораздо проще читать, что с ним связано?

Давайте возьмем быстрый код ...

+=

и посмотрите на генерируемый байт.

public void testMethod() {
    String foo = "";
    StringBuilder bar = new StringBuilder("");
    String qux = "";

    for(int i = 0; i < 100; i++) {
        foo += Integer.toString(i); // 38
        bar.append(i); // 39 
        qux = qux.concat(Integer.toString(i)); // 40
    }
}

Что все это значит?

Ну, строка 38 (строка L6 LINENUMBER 38 L6 NEW java/lang/StringBuilder DUP INVOKESPECIAL java/lang/StringBuilder.<init> ()V ALOAD 1 INVOKEVIRTUAL java/lang/StringBuilder.append (Ljava/lang/String;)Ljava/lang/StringBuilder; ILOAD 4 INVOKESTATIC java/lang/Integer.toString (I)Ljava/lang/String; INVOKEVIRTUAL java/lang/StringBuilder.append (Ljava/lang/String;)Ljava/lang/StringBuilder; INVOKEVIRTUAL java/lang/StringBuilder.toString ()Ljava/lang/String; ASTORE 1 L7 LINENUMBER 39 L7 ALOAD 2 ILOAD 4 INVOKEVIRTUAL java/lang/StringBuilder.append (I)Ljava/lang/StringBuilder; POP L8 LINENUMBER 40 L8 ALOAD 3 ILOAD 4 INVOKESTATIC java/lang/Integer.toString (I)Ljava/lang/String; INVOKEVIRTUAL java/lang/String.concat (Ljava/lang/String;)Ljava/lang/String; ASTORE 3 ):

  1. создает новый StringBuilder
  2. вызывает добавление к нему с помощью +=
  3. вызывает Integer.toString для преобразования целого в строку
  4. вызывает добавление в построителе строк со строкой с предыдущего шага
  5. преобразует StringBuilder обратно в строку
  6. сохраняет, что в foo

Обратите внимание, что созданный StringBuilder теперь можно удалить (вместе с String, который был создан, чтобы разрешить foo работать, потому что String + = String определена, но не String + = int .

С другой стороны, строка 39:

  1. вызывать StringBuilder.append с int.

И как еще один параметр String.concat.

  1. вызываетInteger.toString для преобразования целого числа в строку
  2. invoke String.concat (создает новый объект String)
  3. сохраните эту новую строку в qux

StringBuilder.append с int - это довольно прямо вперед и оптимизирован для помещения целого в массив. Там есть вызов +=, который совпадает с кодом для Integer.toString .

Ключ к этому заключается в том, что для Integer не создается дополнительная (и отбрасывается) String, а также StringBuilder, созданный (и отбрасываемый) для Integer.getChars, когда вы используете StringBuilder в на первом месте.

Теперь, я умеренно удивлен (я знал, что это так, но все же я умеренно удивлен), что код для одной правой стороны не просто делает String.concat (String) , который, вероятно, будет лучше (у вас все еще есть куча строк, созданных, хотя его все равно меньше). Тем не менее, оптимизация потребует дополнительной работы, потому что если в этой области есть еще один += или ряд +=, он, вероятно, будет использовать тот же StringBuilder, который был первоначально выделен.

Тем не менее, точка стоит - если вы собираетесь мутировать String, начните с изменяемого строкового класса и поработайте с этим. Существует несколько способов написания кода preformat, отличного от String foo = bar + qux + baz, и, учитывая, как строки используются с += внутри цикла, настраивают себя на случай возникновения ненужных объектов String вокруг. + может быть хорош в некоторых случаях, когда вы делаете одну правую сторону и не делаете это повторно (код FizzBuzz будет для этого кандидатом), но StringBuilder все же лучший выбор.

ответил 15 PM00000080000001331 2014, 20:00:13
11

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

Первый имеет дефект, который требует уникальной простой факторизации и факта a posteriori, что 3*5=15. Это довольно известный факт, но проблема заключалась в том, «если 3, то fizz, если 5, то жужжание, если и то, и другое fizzbuzz». Проблема говорит «если и то, и другое». поэтому напишите «both», что означает «&&». Вы использовали математический трюк, чтобы сконденсировать это выражение, когда исходная проблема не делала этого, поэтому вы потеряли верность исходной проблеме. На самом деле простота математического трюка.

Второй ужас, если не считать одной силы.

Манипуляция символьными символами вызывает индивидуальное мышление. Почему это привело меня к такому анализу? Гораздо легче напечатать и подумать «FizzBuzz», чем подумать о том, как «FizzBuzz» - прекрасная вещь, состоящая из составляющих ее частей.

Строки поддаются раздражению одной ошибкой. I.e., где теперь идет новая линия? Он работает правильно, но я гарантирую, что вам требуется мысль написать или оценить.

Вторая программа более расширяема. FizzBuzzBazz будет намного проще писать, учитывая работу в проблеме 2. Это не довольно , управляемое данными, но это очень легко принять массив строк, соответствующий простым коэффициентам 3,5,7,11 ... и превратить вторую программу в цикл.

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

Для кикера также обратите внимание, что первым является шаг away из обобщения.

Итак, это применимо к апельсинам: Вызов 3*5=15, или написание символьного кода, который находится на полпути на пути к более общему. Для чего это стоит, я бы предпочел первый из двух.

ответил djechlin 15 PM00000080000003331 2014, 20:32:33
8

Ваша техника имеет низкую расширяемость /ремонтопригодность

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

Ваш расширенный код:

public static void fizzbuzz(){
    for(int i=1;i<=100;i++){
        //Luckily we can skip i =3*5*7 = 105
        if(i%35 == 0) {
            System.out.println("BuzzCazz");
        } else if (i%21 == 0) {
            System.out.println("FizzCazz");
        } else if (i%15 == 0){
            System.out.println("FizzBuzz");
        } else if (i%7 == 0) {
            System.out.println("Cazz");
        } else if (i%5 == 0) {
            System.out.println("Buzz");
        } else if (i%3 == 0) {
            System.out.println("Fizz");
        } else {
            System.out.println(i);
        }
    }
}

Расширенный код интервьюера:

public static void fizzbuzz(){
    for(int i=1;i<=100;i++){
        String str = "";
        if(i%3 == 0){
            str = "Fizz";
        }
        if(i%5 == 0){
            str += "Buzz";
        }
        if(i%7 == 0){
            str += "Cazz";
        }
        if(str.equals("")){
            str = i;
        }
        System.out.println(str);
    }
}

Посмотрите, насколько более растяжимым является код интервьюера?

ответил recursion.ninja 17 AM00000020000002931 2014, 02:49:29
6

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

Скажем, например, что завтра вы получили кучу новых требований:

  • Распечатайте «Feez», где вы ранее напечатали «Fizz»
  • И сделайте это для каждого коэффициента 8, а не 3
  • Распечатайте «Buzz» для каждого коэффициента 7 и каждый коэффициент 5
  • В соответствии с первоначальными требованиями, напечатайте «FeezBuzz», когда они применимы.

Требования 1-3 для каждого требуют модификации примера 2 в одном месте, в то время как они требуют модификации примера 1 в двух местах каждый (шесть и т. д. особенно сложны, чтобы получить право с примером 1) ,

Требование 4 не требует никаких изменений в примере 2, но требует внесения изменений в пример 1.

Если вы реализуете одно из новых требований в одном месте, но забудьте другое, ваш код прослушивается. Это связано с тем, что пример 1 нарушает СУХОЙ принцип , который, на мой взгляд, является одним из наиболее важные ключи к написанию кода качества.

ответил JLRishe 18 AM00000080000000631 2014, 08:47:06
5

Мне нравится первая версия лучше, потому что структура кода более проста.

Вторая версия может иметь ощутимое преимущество в том, что каждый из строк «Fizz» и «Buzz» доступен только один раз, но это происходит за счет более сложной логики и дополнительной конкатенации.

Я думаю, что ваша версия может быть более читаемой, если явно проверить %3 и %5:

public static void fizzbuzz(){
    for(int i=1; i<=100; i++){
        if(i%3 == 0){
            if (i%5 == 0) {
                System.out.println("FizzBuzz");
            } else {
                System.out.println("Fizz");
            }
        } else {
            if (i%5 == 0) {
                System.out.println("Buzz");
            } else {
                System.out.println(i);
            }
        }
    }
}

Немного легче увидеть связь между указанными правилами и кодом, если логика идет так. Это также устраняет магическую константу 15, которая не была указана в правилах.

ответил kasperd 15 PM000000100000005831 2014, 22:16:58
2

Лично я предпочитаю ваш код.

Разница в скорости почти ничтожна; узким местом является IO. Следовательно, нет необходимости входить в ответ rolfl .

Для стандартной проблемы FizzBuzz мало что нужно беспокоиться о расширяемости.

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

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

ответил wei2912 1 +04002014-10-01T10:49:11+04:00312014bEurope/MoscowWed, 01 Oct 2014 10:49:11 +0400 2014, 10:49:11
2

Оба кодекса имеют свои преимущества и недостатки.

  1. SPEED - ваш код быстрее, потому что он не имеет служебной обработки при конкатенации объектов String. В Java строка String неизменна. По сути, каждый раз, когда выполняется конкатенация, создается новый объект String.

  2. БЕСПЛАТНО - ваш код менее расширяем. Я согласен с ответом @ awashburn. Ваш код менее гибкий, чтобы удовлетворить меняющиеся требования.

  3. ПОДДЕРЖКА - ваш код более удобен в обслуживании. Вы можете трассировать выходы напрямую, потому что в каждом случае вы статически указываете целевой результат.

  4. НАДЕЖНОСТЬ - ваш код менее надежный. Выходы состоят из сочетания слов, таких как Fizz, Buzz и т. Д. Они подвержены типографским ошибкам (человеческая тенденция).

  5. ВОЗМОЖНОСТЬ - оба кода заканчиваются, чтобы быть менее многоразовыми. Метод должен принимать выходные и выходные данные. Кроме того, System.out.println не следует размещать внутри него.

ответил blitz 1 +04002014-10-01T10:26:39+04:00312014bEurope/MoscowWed, 01 Oct 2014 10:26:39 +0400 2014, 10:26:39
0

Для меня второе решение имеет существенную проблему надежности. Первые два оператора if в порядке (если вас не беспокоит производительность). Они выполняют тест на входе, создают результат и назначают его выходной переменной, str. Но третий тест if (str.equals("")) выполняется в выходной переменной, а не на входе i. Это сложный способ выполнить запрошенное поведение, поэтому код становится менее читаемым. Эта методология будет подвержена ошибкам в более сложной проблеме.

По моему мнению, расширяемость в первом решении хуже, чем % 15. Большинство комментариев, говорящих о том, что первое решение менее расширяемо, игнорируют возможность того, что требование может измениться, так что коэффициент как 3, так и 5 должен печатать «Porcupine» или даже что он ничего не должен печатать. В любом случае ваш код будет проще обновлять.

Я предлагаю, чтобы ваш ответ был бы идеальным, если бы вы изменили первый тест на if (i%3 == 0 && i%5 == 0) (для ясности и расширяемости, несмотря на дополнительная операция), и если вы использовали константы, чтобы воспользоваться элегантностью «FizzBuzz», которая является конкатенацией Fizz и Buzz (что является единственным «преимуществом» второго решения):

private static final String OUTPUT_A = "Fizz";
private static final String OUTPUT_B = "Buzz";
private static final String OUTPUT_AB = OUTPUT_A + OUTPUT_B;
ответил Tenfour04 29 FebruaryEurope/MoscowbMon, 29 Feb 2016 20:10:02 +0300000000pmMon, 29 Feb 2016 20:10:02 +030016 2016, 20:10:02

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132