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

    +72

    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
    16. 16
    17. 17
    if("all".equalsIgnoreCase(lvl)) {
            setLevel(SimpleLog.LOG_LEVEL_ALL);
        } else if("trace".equalsIgnoreCase(lvl)) {
            setLevel(SimpleLog.LOG_LEVEL_TRACE);
        } else if("debug".equalsIgnoreCase(lvl)) {
            setLevel(SimpleLog.LOG_LEVEL_DEBUG);
        } else if("info".equalsIgnoreCase(lvl)) {
            setLevel(SimpleLog.LOG_LEVEL_INFO);
        } else if("warn".equalsIgnoreCase(lvl)) {
            setLevel(SimpleLog.LOG_LEVEL_WARN);
        } else if("error".equalsIgnoreCase(lvl)) {
            setLevel(SimpleLog.LOG_LEVEL_ERROR);
        } else if("fatal".equalsIgnoreCase(lvl)) {
            setLevel(SimpleLog.LOG_LEVEL_FATAL);
        } else if("off".equalsIgnoreCase(lvl)) {
            setLevel(SimpleLog.LOG_LEVEL_OFF);
        }

    Внутренности конструктора org.apache.commons.logging.impl.SimpleLo g

    Запостил: POPSuL, 30 Октября 2012

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

    • ava 3.14159265 30.10.2012 14:46 # 0

      Ну я в логгерах и похуже код видел.
      Ответить

    • ava roman-kashitsyn 30.10.2012 15:01 # +2

      Намного ли лучше было использовать хэш-таблицу для этого? Тут даже память экономят с equalsIgnoreCase (библиотечный код всё-таки). Апачевский код никогда красотой не отличался.
      Ответить

      • ava POPSuL 30.10.2012 15:04 # +1

        Можно было использовать: для экономии памяти и скорости сравнения - int (флаги вида public static final int LOG_LEVEL_OFF = 100500;), для красивости - switch()
        Ответить

        • ava roman-kashitsyn 30.10.2012 15:08 # 0

          > для красивости - switch()
          по строкам? на входе же строка, которой нужно сопоставить уровень логгирования, при чём здесь int и switch?
          Ответить

          • ava POPSuL 30.10.2012 15:19 # 0

            Как уже сказали ниже - проблема в дизайне/архитектуре. lvl можно было так же представить int'ом, а не строкой. И создавать инстанс кодом вида new SimpleLog(SimpleLog.LOG_LEVEL_TRACE) вместо new SimpleLog("trace"); ну или вообще запилить статические методы вида SimpleLog.createTraceLog(); или что-то вроде.
            Ответить

            • ava roman-kashitsyn 30.10.2012 15:22 # +1

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

          • ava absolut 31.10.2012 06:20 # +1

            а шарпик может и по строкам
            Ответить

      • ava 3.14159265 30.10.2012 15:13 # +2

        Тут ты прав. Во всем.
        Трабла в кривом дизайне lvl задавать строкой - некошерно. Выход набор констант-интов или енумы.
        Но дело ведь в том что пейсалось это в суровые годы JDK14, когда никаких енумов не было.
        Ответить

        • ava roman-kashitsyn 30.10.2012 15:16 # +2

          > Трабла в кривом дизайне lvl задавать строкой - некошерно
          Они же вроде уровень из конфигурационного файла получают, там кроме строки ничего не вытянешь
          Ответить

        • ava absolut 31.10.2012 06:24 # 0

          "пейсалось это в суровые годы" напоминает мне о коде C, опубликованном в разделе C++.
          Ответить

      • ava WGH 30.10.2012 17:25 # 0

        Почему сразу хеш? Можно тупой массив с линейным поиском. По сути, то же самое, что и здесь, но без копипаста.
        Ответить

        • ava roman-kashitsyn 30.10.2012 17:36 # 0

          ну да, как вариант - сделать два статических массива и проходить по ним в цикле. Кода примерно столько же +\- строчка.
          Ответить

    • ava wvxvw 30.10.2012 17:32 # 0

      А если lvl не тот ни другой и не третий - то нафиг этот лог печатать? Так что ли?

      ЗЫ. Однозначно не хватает мультидиспетчеризации на основе сопоставления с образцом! А как бы было замечательно.
      Ответить

      • ava vistefan 16.11.2012 19:34 # +3

        > не тот, ни другой и не третий
        Взлёты и падения...
        Ответить

    • ava Lure Of Chaos 31.10.2012 00:28 # +1

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

      • ava bormand 31.10.2012 06:04 # 0

        Ну хэш был бы всяко не медленнее. Но да, для кода, который исполняется 1 раз за все время работы говорить о времени как-то бессмысленно.
        Ответить

        • ava Lure Of Chaos 31.10.2012 08:53 # 0

          внутренне строки по хэшу сравниваются, то же самое
          Ответить

          • ava bormand 31.10.2012 09:32 # 0

            Так тут же equalsIgnoreCase...
            Ответить

            • ava Lure Of Chaos 31.10.2012 10:15 # 0

              сравниваются внутренние представления, они ловеркейсные и еще там как-то нормализированы
              Ответить

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