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

    +78

    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
    18. 18
    19. 19
    20. 20
    21. 21
    22. 22
    23. 23
    24. 24
    25. 25
    /**
    	* @param loginName
    	* @return
    	* This method is create a LoginName as Input data
    	*/
    public String getLoginName(String loginName)
    {
    	String userQuery="select u.LoginName from User u";
    	Recordset rs_user=null;
    	rs_user = CustomExternalServiceImplUtil.getInstance().executeQuery(userQuery);
    	List<String> userList = new ArrayList<String>();
    	while(rs_user.moveNext()){ 
    		userList.add(rs_user.valueFromIndex(0).toString());
    	}
    	int i=1;
    	String result = loginName;
    	for(int j=0; j < userList.size(); j++){
    		if(userList.get(j).equals(result))
    		{
    			result = loginName+i++;
    			j=0;
    		}
    	}
    	return result;
    }

    Рефаткоринг чужого кода. Минут пять втуплял, что же тут вообще делается. Еще столько же придумывал, как же это привести в божеский вид с сохранением прежней функциональности.

    Запостил: askell, 06 Сентября 2011

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

    • 1. запрос вынести в слой бд
      2. использовать уточнение where
      Ответить
    • И тут залогинилось 2.000.000 пользователей...
      Ответить
    • CustomExternalServiceImplUtil - великолепное название для класса. Сразу понятно, что он выполняет запросы в БД.
      Ответить
      • Жабистов хлебом не корми, дай назвать классы позаковыристей.

        Странно, что не CustomExternalServiceImplUtilFactoryProv ider.getInstance().getCustomExternalServ iceImplUtilFactory().createCustomExterna lServiceImplUtil()
        Ответить
    • Неужели вы не используете Spring Framework?
      Ответить
      • Autumn на дворе
        Ответить
      • А толку?

        Вообще, конечно, мне непонятно, зачем использовать Recordset, когда в JPA есть удобные getResultList и getSingleResult.

        А ещё лучше использовать QueryDSL и не хранить запросы строками.
        Ответить
        • Использование JPA - шаг, на который, возможно, архитекторы не пошли. А вот для JDBC в Spring есть канонический JdbcTemplate и ещё куча всяких плюшек типа декларативных транзакций.
          Ответить
          • Как это не пошли?

            String userQuery="select u.LoginName from User u";

            Это что, не JPQL?
            Ответить
            • Я думаю, это обычный SQL. Тут нам ответит только автор.
              Ответить
              • Хм. И правда, это допустимый SQL, хотя и необычный (что мешает написать просто "SELECT LoginName FROM User"?). Но на JPQL очень уж похоже.
                Ответить
                • Это собственный велосипед данной системы - OQL (Object Query Language). Та еще поделуха...
                  Ответить
                  • То-то я смотрю, u.LoginName не похоже на стандартный доступ к полю (не тот camelCase).
                    Ответить
        • > использовать QueryDSL
          В JPA 2 есть Criteria API, хотя критерии Hibernate мне кажутся более простыми
          Ответить
          • Есть-то он есть, но он там такой, что лучше бы его не было. Страхолюдный и перегруженный API.

            QueryDSL значительно удобнее. И естественным синтаксисом, и отделением параметров запроса от реализации.
            Ответить
            • Посмотрел демку - вроде хорошая штука. Попробую, спасибо за наводку.
              Ответить
          • Hibernate идёт лесом однозначно.
            Ответить
        • Помню, в одном проекте хранил (JP/S)QL-запросы в отдельных файлах с соответствующим расширением (ура, подсветка синтаксиса и валидация) и загружал их кэширующим загрузчиком ресурсов, после чего скармливал entityManager'у. Конечно, у этого решения есть серьёзные недостатки (нет валидации при старте Jpa provider'а, необходимость аккуратной реализации загрузчика).
          Ответить
      • Spring - это который для программирования на XML с конфигами на Java?

        Очень надо такое счастье. Guice + Guice-Persist наше всё.
        Ответить
        • Spring давно уже можно конфигурить аннотациями (включая аннотации jsr-330) и/или Java-кодом, конфигов в этом случае нужен минимум. Guice очень хорош, особенно, когда нужен лёгкий IoC/AOP контейнер. Но Spring давно уже вышел за рамки IoC и AOP, он предоставляет функционал, который Guice предложить не может (иначе бы Guice разросся до размеров Spring). Холивары на эту тему разводить бессмысленно, ибо оба фрэймвёрка чертовски хороши, каждый для своего круга задач. Кстати, авторы Guice очень уважают Spring.
          Ответить
        • пардон, а вы в Tapestry5 не заглядывали? я как увидел - влюбился.
          Ответить
          • думаю, что ещё за модификация Тараса ...
            Ответить
          • Что именно вас так впечатлило? Признаюсь, первый взгляд на Playframework произвёл на меня большее впечатление.
            Ответить
            • 1. использование аннотаций вместо расширения, интерфейсов или xml-конфигурации (на тот момент еще не все проекты повально переходили на аннотации) для всего-всего, поэтому можно назвать метод как угодно, была бы правильная аннотация.
              1.1. поэтому можно писать класс как угодно, наследовать от чего угодно, и использовать повторно, тестировать без поднятия фреймворка, нет конфликтов имен через наследуемые классы и интерфейсы
              1.2. Dependency Injection через аннотации вместо явного управления
              1.3. если что-то поменялось, ничего не надо переименовывать - достаточно изменить аннотацию
              2. идеология компонентов (компонент=1 класс + 1 шаблон)
              2.1 возможность держать всё, относящееся к компоненту (класс, шаблон, ресурсы) в одной папке, а не разворачивать всю глубину соседнего дерева templates, resources или assets
              2.2. запросто локализуемые ресурсы (достаточно добавить префикс локали к имени)
              2.3. инстанс класса на сессию (для каждого пользователя свои поля класса)
              2.4. в режиме разработки нет необходимости перезапускать аппликацию, изменения применяются сразу же
              3. язык шаблонов: шаблон представляет из себя xml-файл без строгой схемы, фреймворк реагирует только на теги и аттрибуты в собственном неймспейсе, а точнее - каждый такой тег представляет собой компонент со своими событиями; аттрибуты приходят в обработчик после байндинга как параметры метода.
              3.1. поэтому шаблоны выглядят очень просто, шаблон не устрашает всякими проверками
              3.2. шаблон пассивен (код не включишь)
              3.3. шаблон можно отдать даже посредственному верстуну, не слыша его ворчаний по поводу непонятного кода в шаблонах
              3.4. даже в визуалках нет такой каши, как в Smarty-подобных шаблонах
              4. нативная интеграция JPA, Hibernate, Spring, Acegi и др.
              5. сборка через Maven
              6. Дока в порядке, фреймворк учится за 5 минут, хорошее сообщество
              может, кое-что и упустил, но это важные свойства для меня, как разработчика.
              Ответить
              • Локализация ресурсов - преимущество ResourceBundle, а не Tapestry лично. А так вроде симпатичная штука. Ради интереса пробовал Spring MVC + Tiles 2, но конфиги tiles немного напрягают.
                На работе суровый JSF 1.2 =(
                Ответить
                • суровый JSF суров, особенно по части именования всего своими именами....

                  разве же ResourceBundle сам понимает, что, если мы указали файл flag.jpg, то, если текущий язык русский, то сначала искать flag_ru_RU.jpg, потом flag_ru.jpg, потом flag_en.jpg, а потом уж flag.jpg? кажется, нужно этот механизм писать самому - в тапестри это поведение по умолчанию
                  Ответить
                  • А, вы вот какие ресурсы имели в виду. Да, для этого нужны некоторые телодвижения.
                    Ответить
                    • да, ресурсы, ассеты, шаблоны и всё остальное...

                      кстати, в альфа-версии 5.3 там уже появилось еще одно измерение ресурсов, называемое skin или theme - c той же лёгкостью можно для тех же компонентов создать мобильную версию, например, не дублируя код.
                      Ответить
                • В общем, я считаю этот фреймворк достойным внимания. Попробовали - не отвечает вкусам - забросили.
                  Правда, есть опасность, что, если тапестри понравился, потом начинаешь плеваться от всех других фреймов =(
                  Ответить
            • есть и минусы:
              1. раньше тапестри был довольно прожорлив (128М котенку не хватало, нужно было 300-400). Это было в версии 5.0, ща вроде бы получше с этим
              2. в режиме разработки раньше котенок очень часто падал с OutOfMemoryError: PermGen space - это пофиксили в версии 5.2
              3. не нашел никакой CMS на тапестри, только фреймворк
              4. АБСОЛЮТНО НИКАКОЙ кодогенерации, всё пишем исключительно ручками :( даже в Playframework это есть...
              5. хорошая поддержка IDE Idea и Netbeans, практически из коробки, а вот мой любимый Eclipse обошли - первоначальная настройка похожа на танцы с бубном - поэтому для новых проектов клонирую самодельный helloworld
              Ответить
              • Как оно в сравнении с Wicket?
                Ответить
                • да, Wicket довольно похож, и хороший конкурент, но есть свои и плюсы и минусы:

                  1. в Tapestry javascript придётся писать самому, нет его генерации на основе кода Java. Зато благодаря стандартным компонентам, это придётся делать не так часто
                  2. шаблоны - строго XML, а не HTML
                  3. нет понятия наследования разметки
                  3. не надо расширять стандартные базовые компоненты (хотя - можно), вся логика работы строится по аннотациям, либо по соглашениям именования

                  4. Tapestry IoC есть предлагает и свои, и возможность создавать т.н. сервисы - невизуальные компоненты, к которым можно обращаться из любого места
                  5. Tapestry много заимствовал из AOP - есть декораторы (теперь рекомендуются к использованию т.н. "советчики"), с помощью которых можно расширить или заместить, а точнее, обернуть, любой метод или сервис. В Wicket я такого не нашел
                  6. Помимо компонентов, есть возможность создавать т.н. "примеси"(mixins) - фичи, которые могут быть использованы в любом компоненте - например, что бы при нажатии на ссылку или кнопку выдавалось подтверждение. Есть ли миксины в Викете? не слышал
                  7. Склейка жабаскриптов и стилей в один файл, хотя на каждый компонент может быть привязаны свои маленькие кусочки через аннотации.

                  8. Ах да, к недостаткам Tapestry: он для клиентской части использует Prototype+Scriptalicious, поэтому на данный момент JQuery можно использовать только в режиме noConflict. Однако, Говард обещал сделать абстрактный слой, который будет работать с любым js-фреймворком - и действительно, в SVN появились таковые наработки, они будут в стабильной 5.3 версии

                  вообще, пишут вот что: https://cwiki.apache.org/WICKET/for-tapestry-users.html
                  Ответить
            • Playframework мне сначала тоже понравился, но перестал пользоваться им когда выяснилось:
              1. шаблонизатор там только Smarty-подобный
              2. stateless-идеология напоминает скорее PHP, нет удобства связаннного в постоянном кручении процесса аппликации...
              Ответить
    • А "WHERE u.LoginName LIKE %{loginName}%", чтобы массив сократить, не?
      Ответить
      • Этот "массив" вообще не нужен. Можно получить результат одним запросом.
        Ответить
    • Set<String> userList = new HashSet<String>();
      
      	while (rs_user.moveNext()) { 
      		userList.add(rs_user.valueFromIndex(0).toString());
      	}
      
      	String result = loginName;
      
      	for(int i=1; ; i++) {
      		if (!userList.contains(result)) {
      			return result;
                      }
      
      		result = loginName + i;
      	}


      Фиксед?
      Ответить
      • Присутствуют 4 необязательные символа операторных скобок =(
        Ответить
        • Я всегда даже одиночные инструкции заключаю в фигурные скобки. Привычка. К тому же не придётся дописывать скобки, если нужно будет вставить ещё инструкции.
          Ответить
          • Потому они и необязательные, а не ненужные.
            Ответить
          • Скобки бывают очень полезны при наличии вложенных ифов, когда без них трудно понять, к какому ифу относится очередной else.
            Ответить
        • Забыл ещё про необязательные пробелы и отступы.
          Ответить
      • Разве конструкция
        for(int i=1; ; i++) {
            if (!userList.contains(result)) {
                return result;
            }
            result = loginName + i;
        }
        не выглядит более естественно, если записать её так:
        int i = 0;
        while (userList.contains(result)) {
            result = userLogin  + (++i);
        }
        ?
        Ответить
        • Конечно, выглядит. Лично я учил цикл с пред/пост условием на рекурсии и ваш вариант кажется мне логичным, нежели использование для подобного цикла со счетчиком и точкой останова посредством return или брэйка (или, о, святая наивность, goto)
          Ответить
        • нет
          Ответить
        • Не эквивалентны. Но выглядит интереснее ;)
          Ответить
        • =)
          Ответить

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