1. Java / Говнокод #10721

    +102

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    12. 12
    13. 13
    14. 14
    15. 15
    public void set...(...){
      ...
      throw INVALID_PARAM;
    }
    public void set...(...){
      ...
      throw INVALID_PARAM;
    }
    public void set...(...){
      ...
      throw INVALID_PARAM;
    }
    
    ...
    private final static Exception INVALID_PARAM=new RuntimeException("Incorrect value!");

    Запостил: 3.14159265, 08 Июня 2012

    Комментарии (70) RSS

    • А что такого? Кроме того, что, наверное, должен быть не RuntimeException, а какой-нибудь стандартный InvalidFunctionParameterOrPropertyValueE rrorException
      Ответить
      • Ага, очень приятно при вызове метода видеть стектрейс, идущий из конструктора, а не из вызванного метода.
        Кроме того, юзабельность нулевая: что за параметр? какое было значение? какое должно было быть?
        Ответить
        • В джаве стектрейс идет от new Exception, а не от throw? Тогда, конечно, лучше это оформить функцией.
          throw ParameterException("theName", value);
          Ответить
          • зачем функция, если есть конструктор?
            Ответить
          • Функция не нужна. Если нужно кидать исключение с особым набором параметров - имхо лучше породить свое исключение с удобным конструктором, и кидать его через throw new BlackjackAndSlutsException(blackjack, sluts).

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

            Конечно же, это мое имхо, а не рекомендации вам.
            Ответить
      • java.lang.IllegalArgumentException?
        Ответить
    • иногда использую такую конструкцию
      public IllegalArgumentException newIllegalArgumentException(...) {
         return new IllegalArgumentException(...);
      }
      
      ...
      throw newIllegalArgumentException("bla-bla-bla");
      Ответить
      • Зачем?
        Ответить
        • ну, бывают ситуации, когда в одном классе надо кинуть много раз эксепшн одного вида. а потом, надо поменять его на эксепшн другого типа. при таком подходе изменения затронут всего лишь одно место. а если, вдобавок, к данному эксепшену потребуется прицепить дополнительную информацию - то плюсы вообще огромные получаются.
          Ответить
          • http://govnokod.ru/10721#comment141272
            Здесь тоже можно легко сменить тип, от которого порожден эксепшн, и добавить дополнительные поля. Так что функция под сомнением...
            Ответить
            • один из примеров, где функция была бы удобна
              public class PNHException extends RuntimeException {	
              	private User user;
              	public User getUser() { return user; }
              	public void setUser(User user) { this.user = user; }
              	...	
              }
              
              public class ServiceClass {
              	
              	@Inject	private User user;
              	
              	public void doSomething() {
              		...
              		try {
              			
              		} catch (IOException e) {
              			throw newPNHException("message 1");
              		}
              		...
              	}
              	
              	public void doSomething2() {
              		...
              		if (!user.canEdit(thing))
              			throw newPNHException("message 2");
              		...
              	}
              	
              	public doSomething3() {...}
              	
              	private PNHException newPNHException(String message) {
              		PNHException e = new PNHException(message);
              		e.setUser(this.user);
              		return e;
              	}
              	
              }
              Ответить
              • Ну для неявного заполнения параметров эксепшена, согласен, функция удобней.
                Ответить
                • а еще и логирование можно всунуть в этот вызов.

                  В идеале вообще можно было бы объявить функцию
                  public void throwNewSomethingException(params) {
                     SomethingException e = ...
                     ...
                     log.error("...", e);
                     throw e;
                  }


                  но к сожалению, это будет не всегда применимо.
                  Ответить
                  • > а еще и логирование можно всунуть в этот вызов.
                    ЗАЧЕМ?
                    не проще ли логгер натравить на stderr (который System.err)?
                    Ответить
              • функции не нужны
                public PNHException(User user, String message) {
                  this(message);
                  this.user=user;
                }
                Ответить
                • а если аргументов 5 или 6?
                  Ответить
                  • тогда бы функция спасла?
                    як дiти (ц)
                    Ответить
                    • Ну поскольку функция, описанная tir в его примере, принимает только message, а остальные параметры заполняет самостоятельно - то спасла бы.
                      Ответить
                      • а, понятно, она выковыривает те поля, которые конструктору были бы не доступны.
                        ну я не вижу проблем конструктору передавать объект со всеми необходимыми данными. можно даже перегрузить сколько надо раз для различных объектов.

                        просто не надо жабе пытаться привить функциональный стиль
                        Ответить
                • давайте попробуем так:
                  public PNHException(String userInfo, String message) {
                    this(message);
                    this.userInfo=userInfo;
                  }


                  а где-то в коде класса идут несколько вызовов
                  throw new PNHException(user.getName(), message)


                  и тут вдруг потребовалось заменить user.getName() на user.getFullName().

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

                  П. С. Если Вы не ленивый человек и в совершенстве владете find/replace, то мы друг друга не поймем =)
                  Ответить
                  • я же написал User user, а не String userInfo.
                    опять же, где функция спасет?
                    Ответить
                    • при использовании функции, мне бы пришлось изменить код в ОДНОМ месте, а не в КАЖДОМ, где бросалось исключение
                      Ответить
                      • ну вот чтобы не менять, и нужно передавать сам объект. ибо имхо везде вызывать user.getName() только для передачи исключению - излишне.
                        а вообще да, пользуйтесь IDEшками и почаще рефакторите код, а не подставляйте странно выглядящие костыли.
                        Ответить
                        • давайте пойдем другим путем, вы считаете такой кусок кода имеющим право на жизнь:
                          private SomeObject createObject(String arg) {
                          		return new SomeObject(this.thing1, this.thing2, this.thing3, arg);
                          	}

                          и далее в коде писать
                          ...
                          createObject("1");
                          ...
                          createObject("2");
                          ...
                          createObject("3");
                          ...

                          или вы сторонник везде в коде писать
                          ...
                          new SomeObject(this.thing1, this.thing2, this.thing3, "1");
                          ...
                          new SomeObject(this.thing1, this.thing2, this.thing3, "2");
                          ...
                          new SomeObject(this.thing1, this.thing2, this.thing3, "3");
                          ...
                          Ответить
                          • лучше конечно new SomeObject(this, ""); этот SomeObject сам решит, сколько ему this.thing нужно.
                            т.е. я предлагаю ту же функцию createObject делать конструктором SomeObject
                            в каждом конкретном случае свои аргументы. иногда можно вообще varargs заюзать
                            Ответить
                            • а если this.thing1 И this.thing2 приватные поля?
                              Ответить
                              • аксессоры, изменить область видимости, на худой конец есть отражение.
                                но честно, за мои 10 лет программинга на жабе мне не приходилось так извращаться. то, что вы описываете, звучит, как задача на олимпиаде или вопрос на собеседовании, т.е. "а если вы в темном подвале, у вас нет компа и интернета, только ручка с бумагой, спросить не у кого, а вам завтра надо сдать проект - как поведет себя сборщик мусора сановской реализации в данных условиях? отвечать быстро, не раздумывая, смотреть в глаза!"
                                Ответить
                                • аксессоры? а как же инкапсуляция и Low Coupling?
                                  Ответить
                                  • ну вот по принципам инкапсуляции исключение и не должно ничего знать о приватных полях = )
                                    Ответить
                                    • вот поэтому оно и ПРИНИМАЕТ на вход значение, а не ЗНАЕТ КАК ЕГО ПОЛУЧИТЬ у конкретного объекта.
                                      Ответить
                                      • а еще лучше заюзать DI, всмысле организовать интерфейс, у которого известно как получить, и передавать его реализацию.
                                        Ответить
                                        • почему же в стандартных эксепшенах нет интерфейсов? Например
                                          IReasonProvider { String getReason(); }

                                          Почему они тупо String принимают?
                                          Ответить
                                          • потому что для reason чаще всего хватает стринга. честно, никогда не видел ничего подобного:
                                            java.io.IOException: Object MyApp, logged as me@localhost:mypass, cannot find file "/tmp/tmp.tmp [rwxrwxrwx]"
                                            вместо этого видим
                                            java.io.IOException: file nound found
                                            caused by
                                            java.io.FileNotFoundException: cannot find file /tmp/tmp.tmp
                                            и как ни странно, этого хватает, несмотря на то, что у нас нет всеобьемлющей инфы о состоянии системы на момент исключения
                                            Ответить
                                            • предлагаю закончить сей разговор =)

                                              П.С. Не удивлюсь, если в вашем коде встречаеццо больше 2-х вызовов геттеров через точку =)

                                              obj.getA().getB().doSomething90
                                              Ответить
                                              • таки да. http://govnokod.ru/8628
                                                Ответить
                                                • не знаком с пхп, но на вид там сеттеры, а я же писал про геттеры ;)
                                                  Ответить
                                                  • ну геттеры тоже встречаются ( вот, нашел, например, такую строку:
                                                    this.setBounds(GraphicsEnvironment.getLocalGraphicsEnvironment().getMaximumWindowBounds());
                                                    Ответить
                                                    • тут тоже не совсем то =)

                                                      скорее я имел ввиду:
                                                      user.getPassport().getPlaceOfBirth().get Country()
                                                      Ответить
                                                      • почему не то?
                                                        вообще да, я так пишу, для разово встречающихся конструкций (избегаю "лишних" временных переменных), но если конструкция встречается более раза, то она сразу рефакторится в локальную (чаще finalную) переменную.
                                                        Ответить
                                                        • а я пишу так
                                                          class User {
                                                          
                                                          public Country getBirthCountry() {
                                                             return getPasport().getPlaceOfBirth().get Country();
                                                          }
                                                          
                                                          }

                                                          или если это нужно конкретно в конектсе условия, то
                                                          boolean isBornIn(Country country) {
                                                             return country.equals(getPassport().getPlaceOfBirth().get Country())
                                                          }


                                                          вызов нескольких геттеров увеличивает свзяанность системы
                                                          Ответить
                                                          • имеет смысл isBornIn тоже унести в User
                                                            Ответить
                                                            • ну, он там и есть, я для краткости не стал объявление класса дублировать
                                                              Ответить
                                                              • ну тогда все норм, и спору нет.
                                                                а так да, иногда встает вопрос, где должна находиться функциональность, имеющая отношение к двум модулям: в первом или втором? чаще ответ бывает: в третьем, новом, модуле.
                                                                Ответить
                  • > В совершенстве владете find/replace
                    В нормальных IDE есть инструменты для подобного рефакторинга.
                    Ответить
                    • да ну нафиг!

                      в какой IDE есть такой рефакторинг: заменить в классе у объектов определенного типа вызов одного метода на другой?
                      Ответить
                      • Так? http://rghost.ru/38576561.view
                        Ответить
                        • да, спасибо, некоторые это умеют. приятно удивлен! =)
                          +1
                          Ответить
                          • eclipse, idea - особенно с хорошо настроенными хоткиями.
                            Ответить
                            • буду признателен, если подскажете где это в eclipse =) сколько лет в нем "кодю", а такого не видел =)
                              Ответить
                              • пардон, дошло, спутал с переименованием метода и извлечением метода. но чаще помогает именно переименование, а потом "ESC" и копипаста переименования обратно.
                                Ответить
                                • Значит я и дальше могу считать QtCreator самой сбалансированной и удобной IDE? :)

                                  Хотя, конечно, для чего-то кроме c++ и qt он малопригоден.
                                  Ответить
                                  • ну для c++ и qt - наверное. а для другого:
                                    Java - Idea, Eclipse,
                                    Javascript - Idea
                                    PHP, Ruby, Python - соотв. продукты от JetBrains или платная версия Ultimate для "все в одном"...
                                    есть еще NetBeans для джавы, но его я давно перестал использовать, т.к. он остал по возможностям даже от еклипса...
                                    Ответить
                      • О, мой друг, IDE сейчас предоставляют возможности рефакторинга куда более мощные, чем эта
                        Ответить
              • По мне так тут попахивает AOP
                Ответить
                • АОП, это близко, но не совсем то.
                  я ждал, но к этому так и не приблизились за всю дискуссию.
                  Ответить
        • Мне нравится подход guava Preconditions
          когда нет возможности использовать guava, пишу аналогичный класс. Сокращается код, намерения становятся явными. А то, что в стек-трейс добавляется строчка вида Preconditions.checkInRange(...), мне не кажется проблемой.
          Фабрика эксепшенов - это вроде как-то черезчур
          Ответить
          • применяю только в случае, когда много однотипных эксепшенов + необходима дополнительная информация (например, к сообщению необходимо добавлять имя текущего пользователя).
            Ответить
    • Ну сколько можно уже изобретать велосипеды, да еще и кривые?

      Вот тут раздают расово-верные велосипеды:
      http://code.google.com/p/guava-libraries/wiki/PreconditionsExplained
      Ответить
      • как всегда ответ один: а если нет гуавы в проекте и подключать не хочется?
        Ответить
        • Идею Preconditions можно и самому без проблем запилить
          Ответить
          • вот и пилим =)
            вот, кстати, кто-то тоже делал
            http://www.java2s.com/Open-Source/Java/CouchDB/ektorp/org/ektorp/util/Assert.java.htm
            Ответить
            • Аналоги есть в Spring, в apache.commons.lang, тысячи их. И очень желательно сделать на входе статических методов не просто строку, а формат + varargs, чтобы конкатенацией постоянно не приходилось заниматься без надобности (лишние вычисления)
              Ответить
              • я специально подбирал пример с функцией выброс эксепшена =)
                Ответить

    Добавить комментарий