Jawa / Говнокод #8365 Ссылка на оригинал

0

  1. 1
  2. 2
  3. 3
  4. 4
  5. 5
  6. 6
  7. 7
  8. 8
  9. 9
  10. 10
  11. 11
  12. 12
  13. 13
  14. 14
  15. 15
  16. 16
  17. 17
  18. 18
  19. 19
  20. 20
  21. 21
public ArrayList<String> bookListByAuthor(String author)
    {
        ArrayList<String> bookList = null;
        for (BookType bType : bookTypes)
        {
            ArrayList<String> authors = bType.getBookAuthors();
            for (String bookAuthor : authors)
            {
                if (author.equalsIgnoreCase(bookAuthor))
                {
                    if (bookList == null)
                    {
                        bookList = new ArrayList<String>(INITIAL_CAPACITY);
                    }
                    bookList.add(author);
                    break;
                }
            }
        }
        return bookList == null ? null : bookList;
    }

Запостил: dwinner dwinner, (Updated )

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

  • О_о. Пишу на курсач ИС Электронная библиотека на JSP + MySQL. Названия похожи. Подход не тот 😀
    return жесткий. Хочу из этого же проекта классы реализующие запросы к БД.
    Ответить
    • Ну да, я собственно из-за return'а и запостил. Если ни одного автора
      не найдется, bookList итак null'ом будем. Ещё забавляет попытка экономии
      памяти с отложенной инициализацией, но это уже другая история...
      Ответить
      • 1. Ещё вместо List возвращается ArrayList, привязка к реализации.
        2. Вообще возвращать null - дурацкая идея, для этого есть Collections.emptyList()
        3. Напрягает алгоритм перебора с квадратичной сложностью
        Ответить
        • 1. Что в этом плохого?
          2. Почему возвращать null плохо?
          3. Как улучшить алгоритм?
          Ответить
          • 1. Скорее всего, придётся менять/как минимум пересобирать код в других местах, когда этот метод будет переписан по-нормальному (for more answers read Bloch)
            2. К примеру, в моём проекте 80% критических ошибок связано с NPE
            3. Использовать для поиска БД
            Ответить
            • мои соображения. в модели используется ArrayList<String>. Выглядит очень странно, но если подумать, то в целом можно придумать обоснование. Я придумал такое: JPA1 поверх гибера. Отсутствует @ElementCollection. Соответственно коллекцию простых типов хрен замапишь. В гибере прокатывает такой вот фокус с ArrayList. Т. к. он Serializable, то сохраниться в базу как blob. И восстанавливается стандартными средствами сериализации. Соответственно, использовать для поиска БД не получится. Отсюда все эти выкрутасы.
              Ответить
              • Руки оторвать дебилу, который хранит авторов в базе как сериализованный список строк
                Ответить
                • в jpa 1 не было другого пути. А создавать для этого отдельную сущность? Спорно.
                  Ответить
                  • > Спорно.
                    Ну да, а ждать, пока нажатие на "Другие книги этого автора" будет выполнять поиск по БД с квадратичной сложностью - не спорно. Ну-ну.
                    Ответить
                    • зависит от размера приложения. не все приложения работают с миллионами записей, некоторые работают с хорошо если с 2-3 тысячами. в таком случае вообще можно всю базу в память спокойно засасывать.

                      П. С. Хотя в данном примере наверное все-таки логично было бы создать сущность. Ибо Пушкин написал далеко не одну книгу.
                      Ответить
                      • Для автора следует создавать отдельную сущность независимо от размера БД. Кто будет следить за тем, что один и тот же автор не встречается два раза под немного разными именами?
                        Ответить
                        • Поэтому я ПС и добавил в предыдущем коменте. Именно из этих соображений.
                          Ответить
            • 1. По этому пункту есть встречный вопрос: а вы всегда в апи, где вам возвращается List делаете копию, если хотите его модифицировать?
              2. В целом соглашусь. Сам предпочитаю никогда не возвращать null. Использую emptyList().
              С другой стороны это может быть логика: либо Null, либо в списке по крайней мере один элемент. Отчасти дело вкуса.
              3. Описал чуть выше, когда это сделать не получится
              Ответить
              • > делаете копию, если хотите его модифицировать
                Зависит от семантики. Однозначного ответа быть не может.
                Ответить
                • вот. а как часто вам приходилось менять тип коллекции, например, с ArrayList на LinkedList?
                  Ответить
                  • Если типом возвращаемого значения будет ArrayList, вернуть Collections.emptyList() уже не удастся.
                    Ответить
                    • Но и в Collections.emptyList() ничего положить уже не получится
                      Ответить
                      • В данном случае класть что-то в результирующий список бессмысленно. Состояние базы таким образом не изменить.
                        Ответить
                        • а может поверх этого списка сразу применяется фильтрация через iterator.remove()? Просто фильтр результатов, никаких изменений в БД и не должно происходить.
                          Ответить
                            • да не, не хочу. просто часто приводят аргумент "можно будет подменить реализацию", а на деле выходит что либо оно не надо, либо хер поменяешь =)

                              Не вижу ничего зазорного в некоторых случаях возвращать конкретную реализацию, вместо интерфейса.

                              П. С. Вообще в беседе пока только мы вдвоем учавствуем как-то =)
                              Ответить
                • вот-вот, семантика. Вообще, по всем правилам, все List надо воспринимать как unmodifiable, если в документации явно не указано обратное. И для добавления или удаления элементов всегда использовать копию.

                  Я, например, иногда пишу небезопасный код: в API возвращается List, а я его модифицирую, зная, что его можно модифицировать, т. к. возвращается копия да к тому же изменяемая.

                  Так почему же возвращать ArrayList - это плохо? Вроде как сразу явная декларация намерений происходит =)
                  Ответить
                  • Разработчик должен всегда оставлять себе пространство для последующего манёвра. Иначе потом могут появиться невзрачные костыли или deprecated методы. Указывая конкретный класс, ты связываешь себя по рукам и ногам. Если завтра придёт адекватный человек и перепишет это безобразие на вызов DAO, придётся создавать новый ArrayList и запихивать туда значения из DAO. А делов-то было - не писать префикс Array.

                    > явная декларация намерений
                    когда я вижу метод, который возвращает ArrayList, я скорее буду воспринимать это как неопытность автора кода, чем как декларацию намерений. И, скорее всего, полезу в сорцы. Не проще ли написать по-человечески и задокументировать семантику?
                    Ответить
                    • может в DAO и не было потребности?
                      И вообще этот больше смахивает на метод модели.

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

                      >написать по-человечески и задокументировать семантику
                      код по сути тоже может являться документацией. ведь юнит-тесты по сути документация.
                      Ответить
        • > Ещё вместо List возвращается ArrayList, привязка к реализации.
          да нет, как раз-таки это нормально - возвращать конкретный подкласс - так больше возможностей манипуляции с ним.
          а вот на вход хорошая практика подавать интерфейсы
          Ответить
      • а return может и не так уж и плох. по коду, конечно, масло масленное, а по читаемости может даже и неплохо. по крайней мере, при беглом взгляде вы сразу сможете увидеть, что может возвращаться как null, так и не null. И не придется для этого просматривать весь код метода, разбираясь где, что и как инициализируется.

        А просто "return bookList;" не дает ответа может ли возвращаться null или нет.
        Ответить
        • Мне и не надо копаться в коде, чтобы это понять. Если я просто пользуюсь чужим методом, то мне достаточно документации.
          Ответить
          • вам ОЧЕНЬ везет с чужим кодом - чаще документации просто нет
            Ответить
            • Я студентота.
              Поэтому, если меня просят что-то посмотреть требую описание методов. Хотя бы на любом сленге, которым обладает индивид. Вот это и есть "документация". Но, сами понимаете, еще надо найти понимающих свою же логику студентов 😀
              Ответить
  • а jpa случайно не поверх гибера? и версия какая jpa?
    Ответить
  • Объясните-ка лучше, почему метод возвращает не список книг, а список с копиями имени автора:
    > bookList.add(author);
    Ответить
    • ща, чувствую, будет пофиксена еще одна бага =)
      Ответить
  • К слову, если уж реализовывать это без обращения к БД, в Scala бы это выглядело примерно так:
    def findBooksByAuthor(author: String) =
      bookTypes.filter(_.authors.find(_.equalsIgnoreCase(author)).isDefined)
    Ответить
    • Хочу прояснить 2 момента
      Оно будет работать раз в 5 медленней, верно?
      Как скала обрабатывает пресловутые nullы?
      Ответить
      • 1. Нет, этот код должен быть практически эквивалентен по скорости выполнения и потребления памяти коду из топика. Можно уменьшить потребление памяти, получив ленивое view коллекции книг.
        2. В Scala не принято вообще использовать null. Пустые списки, как правило, инициализируются значением Nil, что в некотором роде аналогично Collections.emptyList(). Если результат поиска может быть неопределён (find в примере), возвращается инстанс класса Option, который может содержать, а может и не содержать значение (вызов isDefined в примере). Null в любом случае не возвращается.
        Ответить
          • Разве что накладные расходы на вызов виртуального метода объекта-предиката для каждого элемента коллекции. Думаю, JIT сможет это оптимизировать.
            Ответить
    • >_.equalsIgnoreCase
      Что, в данном случае, обозначает символ _ ?
      Глобальное пространство имен?
      Ответить
  • ц:S D P?U$S.A$B$N)ON.L:J!E"U$I N:Q!H:X"J(GL(J HE K G!Y,I"T(Y)O?R:G:C!O)M:H.A(S O)V?Y:T$F"W(P:T.H.F.R"QC(I"A?J)R$Z$R)D(T:Q.A(U D)Y"A"I)G"L!W:E"F L"P?WZ:P(OL)J H:T?C U(O:G.Y$F$IT.S.A.I.W K,N U!M!P,I!K$X.L,B!B"V!F.C.T,O:B(W:J?B,CZ)L?R"I:GF(P A)M.H,H"O)P:Q(M:E)AR,ML.H.E:IШOZWDSHVOYEHKLJDNACEOHRMPMSBIOUPEPDNSLGJEXRZBSHYLJPWTWFEFSFBPKGWSTTLYNJXFWADDBWBPUHVYAUGXZHGSIBJXITDQBGVSVFMLLGDZMFXEBGSPOKSVGTMSAHKTPMUQWQCZRJZGZTRF
    Ответить

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

Помни, guest, за тобой могут следить!

    А не использовать ли нам bbcode?


    8