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

    +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
    18. 18
    19. 19
    20. 20
    21. 21
    22. 22
    23. 23
    24. 24
    25. 25
    26. 26
    27. 27
    28. 28
    29. 29
    30. 30
    31. 31
    32. 32
    33. 33
    34. 34
    35. 35
    36. 36
    37. 37
    38. 38
    39. 39
    40. 40
    41. 41
    42. 42
    43. 43
    44. 44
    45. 45
    46. 46
    47. 47
    48. 48
    49. 49
    50. 50
    51. 51
    52. 52
    53. 53
    54. 54
    55. 55
    56. 56
    57. 57
    58. 58
    59. 59
    60. 60
    61. 61
    @Entity
public class MyObject {
    @Column
    private int type;

    @Id
    private long id;

    @Column
    private String name;

    // и ещё другие поля, а также геттеры-сеттеры для них, в общем, обычная сущность
}

// managed bean в jsp 1.2 (legacy проект)
public class MyList {
    private List<MyObject> oList;
    private SimpleDateFormat filterDateFormat;

    public MyList() {
        filterDateFormat = new SimpleDateFormat("dd-MM-yyyy HH:mm:ss");
    }

    public List<MyObject> getMyList() {
        if (oList == null) {
            oList = DAO.getDAO().findAllMyObjects();
            String name = ...; // берётся из формы
            if (name != null && name != "") {
                oList = getObjectsByName(name, oList);
            }
            Integer type = ...; // тоже берётся из формы
            if (type != null) {
                oList = getObjectsByType(type, oList);
            }
            // и здесь ещё четыре куска такого же говнокода для других свойств MyObject
        }
        return oList;
    }

    private List<MyObject> getObjectsByType(Integer type, List<MyObject> oList) {
        List<MyObject> queriesByType = new ArrayList<MyObject>();
        for (MyObject o : oList) {
            if (o.getType() == type) {
                queriesByType.add(o);
            }
        }
        return queriesByType;
    }

    private List<MyObject> getObjectsByName(String name, List<MyObject> oList) {
        List<MyObject> queriesByName = new ArrayList<MyObject>();
        for (MyObject o : oList) {
            if (o.getName() == name) {
                queriesByName.add(o);
            }
        }
        return queriesByName;
    }

    // и ещё четыре таких же говнометода для других свойств MyObject
}

    Наглядное руководство, как не надо работать с JPA

    Запостил: evg_ever, 28 Января 2014

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

    • ava guest 28.01.2014 12:08 # 0

      А вдруг тут просто экономят на объектах String и Integer. Хранят где-нибудь. А потом очень быстро делают ==.
      Ответить

      • ava VseGovnoOdinYaKrut 28.01.2014 12:18 # +3

        На зарплатах они экономят, на зарплатах.
        Ответить

      • ava evg_ever 28.01.2014 12:26 # 0

        Ага, конечно, экономят на String и Integer, зато не думая фигачат ArrayList, хотя можно было бы LinkedList юзать и не копировать одно и то же из списка в список по сто раз.
        Ответить

        • ava roman-kashitsyn 28.01.2014 12:28 # 0

          Можно про LinkedList и некопирование поподробнее?
          Ответить

          • ava evg_ever 28.01.2014 12:46 # 0

            Конечно :) если возвращать из findAllMyObjects() LinkedList, а не ArrayList (этот метод всё равно нигде не используется за пределами класса MyList), можно переписать говнометоды таким образом:

            private List<MyObject> getObjectsByType(Integer type, List<MyObject> oList) {
    for (Iterator<MyObject> it = oList.iterator(); it.hasNext(); ) {
        if (o.getType() == type) {
            oList.remove(o);
        }
    }
    return oList;
}


            И таким образом получим нормальное удаление элементов из списка, выполняющееся за O(1), тем более, что этот код куда логичнее и понятнее.

            А ещё лучше просто один нормальный JPQL/Criteria запрос было сделать, тогда вообще не нужно было бы плодить все эти методы...
            Ответить

          • ava evg_ever 28.01.2014 12:52 # 0

            Прошу прощения, фигню написал выше. Вот правильный вариант:

            private List<MyObject> getObjectsByType(Integer type, List<MyObject> oList) {
    for (Iterator<MyObject> it = oList.iterator(); it.hasNext(); ) {
        MyObject o = it.next();
        if (o.getType() == type) {
            it.remove();
        }
    }
    return oList;
}
            Ответить

            • ava roman-kashitsyn 28.01.2014 12:58 # 0

              если отвлечься от оптимального варианта с поиском в базе сразу того, что нужно, я бы предпочёл перекладывание из одного ArrayList в другой с одним проходом, в котором комбинируется проверка всех условий.
              Ответить

              • ava evg_ever 28.01.2014 13:16 # 0

                Про 1 проход тоже верно, пожалуй. Только зачем перекладывать из одного в другой, если можно использовать один-единственный объект?

                if ((type == null || type == o.getType() &&
        (name == null || name.equals("") || name == o.getName()) &&
        /* ... */) {
    it.remove();
}


                Блин, там ещё и == для строк вместо equals используется ведь. %) Хотя в данном случае, может, и по фиг, вряд ли Hibernate будет "" новые создавать.
                Ответить

                • ava roman-kashitsyn 28.01.2014 15:53 # 0

                  > зачем перекладывать из одного в другой, если можно использовать один-единственный объект?

                  потому что связные списки тормозные из-за нелокальности ссылок и жрут в несколько раз больше памяти
                  Ответить

                  • ava evg_ever 28.01.2014 16:41 # 0 

                    public static void main(String[] args) throws Exception {
	Random random = new Random();
	Integer[] array = new Integer[100000];
	for (int i = 0; i < array.length; i++) {
		array[i] = random.nextInt(10);
	}
	ArrayList<Integer> arrayList = new ArrayList<Integer>(Arrays.asList(array));
	test(arrayList, array);
	LinkedList<Integer> linkedList = new LinkedList<Integer>(Arrays.asList(array));
	test(linkedList, array);
}

private static void test(List<Integer> list, Integer[] array) throws Exception {
	long begin = System.currentTimeMillis();
	for (Iterator<Integer> it = list.iterator(); it.hasNext(); ) {
		if (it.next().intValue() == 4) {
			it.remove();
		}
	}
	long end = System.currentTimeMillis();
	System.out.println(list.getClass().getSimpleName() + ": " + (end - begin));
}


                    вывод:
                    ArrayList: 426
                    LinkedList: 22

                    тормозные, значит?
                    Ответить

                    • ava roman-kashitsyn 28.01.2014 16:55 # +1

                      Всё неправильно. Я же говорил, копировать из одного ArrayList в другой, а не удалять из исходного.
                      http://ideone.com/ZEiNwy
                      Ответить

                      • ava evg_ever 28.01.2014 17:08 # 0

                        Да, верно, значит, разницы в производительности практически нет. Хотя вариант с LinkedList более логичный, по-моему :)
                        Ответить

                        • ava roman-kashitsyn 28.01.2014 17:12 # 0

                          Кому что нравится. Надо просто не забывать, что LinkedList это как минимум +2 ссылки на каждый хранимый элемент и постоянные кэш-промахи. Иногда на это наплевать, иногда - нет.
                          Ответить

            • ava guest 28.01.2014 13:18 # +2

              if (new Integer(3) == new Integer(3))
              System.out.println("что-то не так с жабой");
              Ответить

              • ava VseGovnoOdinYaKrut 28.01.2014 13:27 # 0

                if (new Integer(31337) != new Integer(31337))
                System.out.println("да не, всё в порядке");
                Ответить

              • ava evg_ever 28.01.2014 13:31 # 0

                Ага

                - if (o.getType() == type)
                + if (o.getType().equals(type))
                Ответить

                • ava roman-kashitsyn 28.01.2014 13:36 # 0

                  Objects.equal
                  Ответить

                  • ava VseGovnoOdinYaKrut 28.01.2014 13:50 # 0

                    В топку гуаву, a == null ? b == null : a.equals(b)
                    Ответить

                    • ava 3.14159265 28.01.2014 13:52 # +1

                      Это из стандартной либы, VseGovnoOdinYaKrut.
                      Ответить

                      • ava VseGovnoOdinYaKrut 28.01.2014 16:05 # +3

                        Из стандартной либы Objects.equals, где s на конце. Да вот незадача, оно только начиная с 7 жабы. А Objects.equal из гуамвы.
                        Ответить

                • ava guest 28.01.2014 17:39 # 0

                  вот поэтому поля с ограниченным набором значений лучше заворачивать в enum, тому пофиг равно или икволс
                  Ответить

    • ava VseGovnoOdinYaKrut 28.01.2014 12:23 # 0

      Что-то мне кажется, что во времена того легаси ещё не придумали никакого jpa и это результат натягивания ужа на ежа. Может они раньше в csv хранили данные и тянули всё в память.
      Ответить

      • ava evg_ever 28.01.2014 12:28 # 0

        Нет, данные изначально хранились в БД. JPA не придумали, Hibernate придумали, он и используется)
        Ответить

    • ava someone 28.01.2014 21:00 # 0

      Ну не знают они про SELECT WHERE, не знают. Зато фильтрация в Java-коде гораздо гибче и может обладать нетривиальной логикой!
      Ответить

      • ava evg_ever 28.01.2014 21:40 # 0

        Не знаю, как в Hibernate, а вот в NHibernate (порт под .net) вроде можно в where добавлять любой метод, возвращающий bool
        Ответить

        • ava 3.14159265 29.01.2014 17:45 # 0

          LINQ
          Ответить

        • ava laMer007 30.01.2014 12:31 # 0

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

    • ava Lure Of Chaos 29.01.2014 16:07 # 0

      ппц, от JPA тут вообще рожки да ножки.
      Ответить

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