Форум программистов, компьютерный форум, киберфорум
Java SE (J2SE)
Войти
Регистрация
Восстановить пароль
Блоги Сообщество Поиск Заказать работу  
 
Рейтинг 4.75/4: Рейтинг темы: голосов - 4, средняя оценка - 4.75
26 / 26 / 6
Регистрация: 09.02.2011
Сообщений: 71

Покритикуйте код

14.06.2017, 10:15. Показов 831. Ответов 13
Метки нет (Все метки)

Студворк — интернет-сервис помощи студентам
Всем привет.

У меня есть обычный POJO-класс с полями и класс для работы с данной коллекцией объектов.
Посоветуйте плс, как улучшить код в стиле Java8.


Вот линк на гитхаб:
https://github.com/VictorSem/P... oductJava8
0
Programming
Эксперт
39485 / 9562 / 3019
Регистрация: 12.04.2006
Сообщений: 41,671
Блог
14.06.2017, 10:15
Ответы с готовыми решениями:

Покритикуйте код моего сокет сервера для игрового чата
С помощью пары уроков, сделал сервер для визуального чата, типа галактики знакомств. Интересны советы по упрощению и оптимизации. Там...

Покритикуйте код
Добрый вечер. Выполнял одно тестовое задание (уже получил отрицательный ответ), буду рад, если скажите все ошибки, а также, как их можно...

Покритикуйте новичка
Доброго всем времени суток! Самоучкой пытаюсь освоить Java, общение с опытными программистами или преподавателями недоступно. Остаётся...

13
 Аватар для Doctor_
238 / 237 / 142
Регистрация: 03.02.2011
Сообщений: 1,437
14.06.2017, 12:48
Посоветуйте плс, как улучшить код в стиле Java8.
Это вообще как понимать? Что это за требование такое?
0
26 / 26 / 6
Регистрация: 09.02.2011
Сообщений: 71
14.06.2017, 13:44  [ТС]
В смысле "как понимать". Есть код, я хочу чтобы посмотрели код, указали на слабые места, что можно улучшить/удалить/добавить и т.п.
0
33 / 33 / 20
Регистрация: 18.03.2016
Сообщений: 101
19.06.2017, 03:05
А не могли бы на сайт код копировать. Вот в таком формате (спойлер под каждый файл):
Кликните здесь для просмотра всего текста
Java
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
public class Main {
    public static void main(String[] args) {
        // Код
    }
    static String restructuring(String[] text) {
        int path = 0;
        text[0] = text[0].substring(0,text[0].length()-1);
        for (int i = 1 ; i < text.length ; i++) {
            if (!text[i].contains(" ")) {
                text[i] = text[i].substring(0,text[i].length()-1);
                text[path] = "";
                path = i;
            } else {
                text[i] = text[i].substring(0,text[i].lastIndexOf(" ")+1) + text[path] + "/" + text[i].substring(text[i].lastIndexOf(" ")+1);
            }
        }
        text[path] = "";
        StringBuilder sb = new StringBuilder("");
        for (String s : text) {
            if (!s.equals("")) {
                sb.append(s);
                sb.append("\n");
            }
        }
        return sb.toString();
    }
}
0
 Аватар для HighPredator
6045 / 2160 / 753
Регистрация: 10.12.2010
Сообщений: 6,005
Записей в блоге: 3
19.06.2017, 08:53
I. Мне не нравится ваш класс Product.
Java
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
public class Product {
    private String name;
    private Integer price;  
    private String brand;
    
    public Integer getPrice() {
        return price;
    }
 
    public Product setPrice(Integer price) {
        this.price = price;
        return this;
    }
    
    public String getName() {
        return name;
    }
    public Product setName(String name) {
        this.name = name;
        return this;
    }
    
    public String getBrand() {
        return brand;
    }
    public Product setBrand(String brand) {
        this.brand = brand;
        return this;
    }
 
    @Override
    public String toString() {
        return "Product [name=" + name + ", price=" + price + ", brand=" + brand + "]";
    }
}
Вы нарушили логику использования сеттеров, причем необоснованно. Плюс, для работы вам абсолютно ничто не мешает принимать все три поля в конструкторе.

II. Класс ProductCollections
Кликните здесь для просмотра всего текста
Java
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
public class ProductCollections {
    private static final Logger log = Logger.getLogger(ProductCollections.class);
 
    private ArrayList<Product> prodCollection = new ArrayList<>();
 
    private Stream<Product> products() {
        return getProdCollection().stream();
    }
 
    public int size() {
        return prodCollection.size();
    }
 
    public Product get(int i) {
        return prodCollection.get(i);
    }
 
    public boolean isEmpty() { 
        return prodCollection.isEmpty();
    }
 
    public ArrayList<Product> getProdCollection() {
        if (isEmpty()) {
            prodCollection.add(new Product().setName("Икра красная "Вкуснота "").setBrand("Наши традиции").setPrice(25));
            prodCollection.add(new Product().setName("Чай зеленый "Принцесса Нури"").setBrand("Ахмад").setPrice(15));
            prodCollection.add(new Product().setName("Чай ароматный с бергамотом.").setBrand("Беседа").setPrice(140));
            prodCollection.add(new Product().setName("Чай ароматный с бергамотом.").setBrand("Беседа").setPrice(65));
            prodCollection.add(new Product().setName("Lipton Yellow Label").setBrand("Lipton").setPrice(50));
            prodCollection.add(new Product().setName("Magic aroma").setBrand("Lipton").setPrice(18));
            prodCollection.add(new Product().setName("English Breakfast").setBrand("Lipton").setPrice(12));
            prodCollection.add(new Product().setName("Royal Ceylon").setBrand("Lipton").setPrice(14));
        }
 
        return prodCollection;
    }   
 
    @Override
    public String toString() {
        return "ProductCollections [prodCollection=" + prodCollection + "]";
    }
 
    public List<Product> sortByName() {
        return products().sorted(Comparator.comparing(Product::getName)).collect(Collectors.toList());
    }
 
    public List<Product> sortByPrice() {
        return products().sorted(Comparator.comparingInt(Product::getPrice)).collect(Collectors.toList());
 
    }
 
    public Product prodWithMaxPrice() {
        return products().max(Comparator.comparing(Product::getPrice)).get();
    }
 
    public Product prodWithMinPrice() {
        return products().min(Comparator.comparingInt(Product::getPrice)).get();
    }
 
    public List<Product> findBy(Predicate<Product> predicate) {
        List<Product> list = products().filter(predicate)
                .collect(Collectors.toList());
 
        if (list.isEmpty()) {
            log.info("Dont find element.");
        }
        return list;
    }   
    
    public static Predicate<Product> productName(String productName) {
        return p -> p.getName().contains(productName);
    }
    
    public static Predicate<Product> productBrand(String productBrand) {
        return p -> p.getBrand().contains(productBrand);
    }
    
    public static Predicate<Product> productPrice(Integer productPrice) {
        return p -> p.getPrice().equals(productPrice);
    }
}
У вас он имеет внутреннее состояние, соответственно: лучше переименовать в ProductCollection например (см. Макконнела или Мартина). Модификаторы доступа вообще жесть: у вас приватный метод products возвращает стрим от аррэйлиста, выдаваемого публичным методом getProdCollection, который по сути геттер. Да еще к тому же и вызывается только там. Отсюда вопрос: а в таком ключе он вообще нужен (риторический если что)? Методы сортировки sortBy-bla-bla: они не используются. Зачем они:
а) вообще
б) конкретно в этом классе (это вообще не его ответственность -- снова см. любого из двух авторов у кого описано разделение ответственностей)
?

Итого: грустно. Три класса, два из которых демонстрируют полную непродуманность процесса. Доводите до ума.
1
Эксперт Java
378 / 370 / 114
Регистрация: 30.06.2010
Сообщений: 1,445
20.06.2017, 21:01
я бы сказал еще что вызов метода get() у Optional - bad practice
0
 Аватар для HighPredator
6045 / 2160 / 753
Регистрация: 10.12.2010
Сообщений: 6,005
Записей в блоге: 3
21.06.2017, 08:26
Цитата Сообщение от LeX Посмотреть сообщение
вызов метода get() у Optional
А где это у ТС-а?
0
Эксперт Java
3639 / 2971 / 918
Регистрация: 05.07.2013
Сообщений: 14,220
21.06.2017, 10:12
Цитата Сообщение от HighPredator Посмотреть сообщение
Вы нарушили логику использования сеттеров
возвращать из сеттеров this на мой взгляд прекрасная идея, или не про это речь?
Цитата Сообщение от HighPredator Посмотреть сообщение
А где это у ТС-а?
Цитата Сообщение от HighPredator Посмотреть сообщение
public Product prodWithMaxPrice() {
* * * * return products().max(Comparator.comparing(Prod uct::getPrice)).get();
* * }
я бы вообще весь класс убрал, возможно предикаты можно оставить.
1
 Аватар для HighPredator
6045 / 2160 / 753
Регистрация: 10.12.2010
Сообщений: 6,005
Записей в блоге: 3
21.06.2017, 11:51
xoraxax, увидел, спасибо.
Цитата Сообщение от xoraxax Посмотреть сообщение
не про это речь
Про это.

Добавлено через 49 минут
Идея в том, что подобные сеттеры:
1) делают код нетривиальным для чтения, ибо, если доков к методу нет например, сразу неочевидно какое состояние объекта возвращается: до или после (а тесты могут и не показывать)
2) ломают javabeans конвенцию
3) фреймворки могут не принять такие сеттеры (следствие 2)
0
Эксперт Java
3639 / 2971 / 918
Регистрация: 05.07.2013
Сообщений: 14,220
21.06.2017, 12:24
Цитата Сообщение от HighPredator Посмотреть сообщение
Идея в том, что подобные сеттеры:
1) делают код нетривиальным для чтения, ибо, если доков к методу нет например, сразу неочевидно какое состояние объекта возвращается: до или после (а тесты могут и не показывать)
2) ломают javabeans конвенцию
3) фреймворки могут не принять такие сеттеры (следствие 2)
в lombok постоянно используем @Accessors(chain=true), все отлично работает, ничего не ломается
0
Эксперт Java
378 / 370 / 114
Регистрация: 30.06.2010
Сообщений: 1,445
21.06.2017, 16:43
я бы сказал еще что вызов метода get() у Optional - bad practice
Цитата Сообщение от xoraxax Посмотреть сообщение
в lombok постоянно используем @Accessors(chain=true), все отлично работает, ничего не ломается
не есть хорошо, надо использовать builder.
Цитата Сообщение от HighPredator Посмотреть сообщение
2) ломают javabeans конвенцию
3) фреймворки могут не принять такие сеттеры (следствие 2)
2 - ломать не ломают, но не соответствуют
3 - не встречал, но по идее не должно, return value может игнорится, хотя... если только фреймворк не жестко следует javabeans

Добавлено через 15 секунд
я бы сказал еще что вызов метода get() у Optional - bad practice
Цитата Сообщение от xoraxax Посмотреть сообщение
в lombok постоянно используем @Accessors(chain=true), все отлично работает, ничего не ломается
не есть хорошо, надо использовать builder.
Цитата Сообщение от HighPredator Посмотреть сообщение
2) ломают javabeans конвенцию
3) фреймворки могут не принять такие сеттеры (следствие 2)
2 - ломать не ломают, но не соответствуют
3 - не встречал, но по идее не должно, return value может игнорится, хотя... если только фреймворк не жестко следует javabeans
0
Эксперт функциональных языков программированияЭксперт Java
 Аватар для korvin_
4575 / 2774 / 491
Регистрация: 28.04.2012
Сообщений: 8,765
21.06.2017, 19:13
Цитата Сообщение от HighPredator Посмотреть сообщение
2) ломают javabeans конвенцию
3) фреймворки могут не принять такие сеттеры (следствие 2)
К чёрту Java Bean и подобные фреймворки. =)
0
Эксперт Java
3639 / 2971 / 918
Регистрация: 05.07.2013
Сообщений: 14,220
21.06.2017, 20:52
Цитата Сообщение от LeX Посмотреть сообщение
не есть хорошо, надо использовать builder.
т.к. до сих пор не было никаких проблем, значит есть хорошо. Как раз таки вместо билдера стали использовать chain, потому что проще и быстрее.
0
Эксперт Java
378 / 370 / 114
Регистрация: 30.06.2010
Сообщений: 1,445
21.06.2017, 21:14
Цитата Сообщение от xoraxax Посмотреть сообщение
Как раз таки вместо билдера стали использовать chain, потому что проще и быстрее.
на 2 вызова метода? а если поле final? или меняется только внутренней логикой? у меня бывает достаточное количество объектов с конструктором и геттерами
0
Надоела реклама? Зарегистрируйтесь и она исчезнет полностью.
inter-admin
Эксперт
29715 / 6470 / 2152
Регистрация: 06.03.2009
Сообщений: 28,500
Блог
21.06.2017, 21:14
Помогаю со студенческими работами здесь

Алгоритм: Трамвайные билеты. Покритикуйте
Привет, решение данной задачи на Java. Покритикуйте. package olympicexercises; import java.io.*; import java.util.*; import...

Задачи на строки и числа. Решение. Покритикуйте
Задачи: Привет, решение данных заданий на Java - см. ссылку ниже. Покритикуйте. import java.io.InputStream; import...

Задачи на числа. Решение. Покритикуйте. (часть №1)
Привет, решение данных заданий на Java - ниже. Покритикуйте. package chapt01.b; import java.util.*; import static...

Покритикуйте пожалуйста резюме Junior Java Developer
Пожалуйста скажите что стоит поправить. Если кому не сложно скиньте свои резюме с которыми вы устраивались. Резюме

Покритикуйте код
Добрый день. Пытался устроиться на работу (C# junior программист, зп 15-20к). По результатам выполнения задания, на работу не взяли....


Искать еще темы с ответами

Или воспользуйтесь поиском по форуму:
14
Ответ Создать тему
Новые блоги и статьи
SDL3 для Web (WebAssembly): Обработчик клика мыши в браузере ПК и касания экрана в браузере на мобильном устройстве
8Observer8 02.02.2026
Содержание блога Для начала пошагово создадим рабочий пример для подготовки к экспериментам в браузере ПК и в браузере мобильного устройства. Потом напишем обработчик клика мыши и обработчик. . .
Философия технологии
iceja 01.02.2026
На мой взгляд у человека в технических проектах остается роль генерального директора. Все остальное нейронки делают уже лучше человека. Они не могут нести предпринимательские риски, не могут. . .
SDL3 для Web (WebAssembly): Вывод текста со шрифтом TTF с помощью SDL3_ttf
8Observer8 01.02.2026
Содержание блога В этой пошаговой инструкции создадим с нуля веб-приложение, которое выводит текст в окне браузера. Запустим на Android на локальном сервере. Загрузим Release на бесплатный. . .
SDL3 для Web (WebAssembly): Сборка C/C++ проекта из консоли
8Observer8 30.01.2026
Содержание блога Если вы откроете примеры для начинающих на официальном репозитории SDL3 в папке: examples, то вы увидите, что все примеры используют следующие четыре обязательные функции, а. . .
SDL3 для Web (WebAssembly): Установка Emscripten SDK (emsdk) и CMake для сборки C и C++ приложений в Wasm
8Observer8 30.01.2026
Содержание блога Для того чтобы скачать Emscripten SDK (emsdk) необходимо сначало скачать и уставить Git: Install for Windows. Следуйте стандартной процедуре установки Git через установщик. . . .
SDL3 для Android: Подключение Box2D v3, физика и отрисовка коллайдеров
8Observer8 29.01.2026
Содержание блога Box2D - это библиотека для 2D физики для анимаций и игр. С её помощью можно определять были ли коллизии между конкретными объектами. Версия v3 была полностью переписана на Си, в. . .
Инструменты COM: Сохранение данный из VARIANT в файл и загрузка из файла в VARIANT
bedvit 28.01.2026
Сохранение базовых типов COM и массивов (одномерных или двухмерных) любой вложенности (деревья) в файл, с возможностью выбора алгоритмов сжатия и шифрования. Часть библиотеки BedvitCOM Использованы. . .
SDL3 для Android: Загрузка PNG с альфа-каналом с помощью SDL_LoadPNG (без SDL3_image)
8Observer8 28.01.2026
Содержание блога SDL3 имеет собственные средства для загрузки и отображения PNG-файлов с альфа-каналом и базовой работы с ними. В этой инструкции используется функция SDL_LoadPNG(), которая. . .
КиберФорум - форум программистов, компьютерный форум, программирование
Powered by vBulletin
Copyright ©2000 - 2026, CyberForum.ru