Code review by Java Professional Developer

@Almaz_1993 · Регистрация: 24.03.2013

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

Ребята, нужен человек с достаточно высоким левелом по Java, для проведения Code review моего проекта.

Проект на гитхабе
Симулятор торговли на фондовом рынке
Начал этот проект недавно, но уже столкнулся с большими архитектурными проблемами. Поэтому прошу, у кого есть немного свободного времени проревьювить код и указать на недостатки и т.д.

Заранее благодарен.

@KEKCoGEN · 15.10.2015, 17:09

Almaz_1993, для того чтобы делать код ревью для такого проекта нужно либо понимать в трейдинге, либо видеть диаграмму классов, их отношений, и flow диаграмму объясняющую что происходит в системе. Можно конечно долго читать код и все это понять, но мне нарпимер жаль на это времени.

Из быстрого просмотра проекта:

* Не храните lib на гитхабе. Мейвен должен сам приносить что надо
* Проект использует (или не использует) некую библиотеку, которой нет в pom.xml. Импорты lt.monarch.* не резолвятся (я так понимаю это UI)
* Все методы довольно длинные что затрудняет чтение и понимание кода. Метод не должен превышеть 10-50 строк
* В программе используется константа DEFAULT_OUTPUT_REPORT_DIRECTORY которая указывает на директорию которой может и не быть. Эта настройка должна быть в проперти файлах или указывать на директорию проекта которая гарантировано существует.

* BackTester tester = new BackTester(strategy, backTestDataProvider); -- зачем передавать провайдер если он есть как поле в стратегии?

* Сам провайдер можно реализовать как итератор. Делать это через переменную currentBarIndex имхо так себе
* В проекте указанно что для него необходима джава 8, однако никакого кода из джава 8 я не увидел (полагаю джава 8 это для javaFX, однако почему бы не писать уже код на джава 8 если она используется)

Насчет архитектуры сказать немогу т.к не особо всматривался, но вроде ничего страшного не увидел.

@Almaz_1993 · 15.10.2015, 18:14 **[ТС]**

Спасибо большое!

Сообщение от KEKCoGEN

* Не храните lib на гитхабе. Мейвен должен сам приносить что надо

Fixed

Сообщение от KEKCoGEN

Проект использует (или не использует) некую библиотеку, которой нет в pom.xml. Импорты lt.monarch.* не резолвятся (я так понимаю это UI)

Да, это UI. lt.monarch - это либа для построения графиков, для которой нет открытой maven directory. Не знаю откуда и куда ставить mvn dependency, т.е. что прописать в dependency. Но этот jar файл есть в /resource/libs

Сообщение от KEKCoGEN

Все методы довольно длинные что затрудняет чтение и понимание кода. Метод не должен превышеть 10-50 строк

Понял, буду править. Много раз читал, что не надо делать длинные методы - буду фиксить

Сообщение от KEKCoGEN

В программе используется константа DEFAULT_OUTPUT_REPORT_DIRECTORY

Will be fixed soon

Сообщение от KEKCoGEN

* BackTester tester = new BackTester(strategy, backTestDataProvider); -- зачем передавать провайдер если он есть как поле в стратегии?

Вот такие архитектурные ошибки и выводят меня из себя. Как так получается - не понимаю, как-то само :-(

Сообщение от KEKCoGEN

Сам провайдер можно реализовать как итератор. Делать это через переменную currentBarIndex имхо так себе

На данный момент этот код этого класса не до конца готов. Этот класс по идее должен выдавать информацию о только текущей и прошлых барах.

Сообщение от KEKCoGEN

В проекте указанно что для него необходима джава 8,

Там, в некоторых моментах юзаются лямбды

Добавлено через 3 минуты

Сообщение от Almaz_1993

зачем передавать провайдер если он есть как поле в стратегии?

А это вообще нормально, что у Strategy есть поле BackTestDataProvider ?
Это же ведь вродь 2 разные сущности и они ни в коем случае не включаться друг в друга?

@KEKCoGEN · 15.10.2015, 18:41

Сообщение от Almaz_1993

это вообще нормально, что у Strategy есть поле BackTestDataProvider

незнаю. Для этого надо точнее понимать архитектуру проекта и назначение каждого класса. Я в это не углублялся.

@Almaz_1993 · 15.10.2015, 19:02 **[ТС]**

Еще не нравится вот такие вещи. Мне кажется такие вещи можно было более элегантнее решить.

Эти значения проперти - используются для репортинга

class BackTester

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
 private BackTestResearchResult getBackTestResult() throws TradingException{
        BackTestResearchResult result = new BackTestResearchResult(this.strategy);
//        result.setProperties(strategy.getProperties());
 
        result.setOrders(strategy.getPositionManager().getClosedOrders());
        result.setStockHistory(backTestDataProvider.getStockHistory());
 
 
        Map<String, Object> properties = result.getProperties();
 
        properties.put("strategyName", this.strategy.getStrategyName());
        properties.put("orders", this.strategy.getPositionManager().getClosedOrders());
        properties.put("grossProfit", TradeUtils.getGrossProfit(result));
        properties.put("grossLoss", TradeUtils.getGrossLoss(result));
        properties.put("totalNet", TradeUtils.getTotalNet(result));
        properties.put("profitFactor", TradeUtils.getProfitFactor(result));
        properties.put("expectedPayoff", TradeUtils.getExpectedPayoff(result));
 
        properties.put("totalTrades", TradeUtils.getCountPositions(result));
        properties.put("totalShortTradeCount", TradeUtils.getShortPositions(result));
        properties.put("totalLongTradeCount", TradeUtils.getLongPositions(result));
        properties.put("totalProfitTradeCount", TradeUtils.getCountProfitTrades(result));
        properties.put("totalLossTradeCount", TradeUtils.getCountLossTrades(result));
 
        properties.put("totalLargestProfit", TradeUtils.getLargestProfit(result));
        properties.put("totalLargestLoss", TradeUtils.getLargestLoss(result));
        properties.put("totalAverageProfit", TradeUtils.getAverageProfit(result));
        properties.put("totalAverageLoss", TradeUtils.getAverageLoss(result));
        properties.put("totalProfitPercentage", TradeUtils.getTotalProfitPercentage(result));
 
        properties.put("consecutiveProfitCount", TradeUtils.getConsecutiveProfitCount(result));
        properties.put("consecutiveLossCount", TradeUtils.getConsecutiveLossCount(result));
        properties.put("defaultOrderSize", strategy.DEFAULT_ORDER_SIZE);
        properties.put("drawdownBalanceClose", TradeUtils.getDrawdownBalanceClose(result));
        properties.put("startDate", TradeUtils.getFirstBar(backTestDataProvider).getDate());
        properties.put("endDate", TradeUtils.getLastBar(backTestDataProvider).getDate());
 
        return result;
    }

@KEKCoGEN · 15.10.2015, 19:45

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

@Almaz_1993 · 15.10.2015, 22:11 **[ТС]**

Проблема еще в том, что не все проперти используются.

Для одного репорт билдера - нужны одни данные, для другого другие.

Новые блоги и статьи Все статьи Все блоги /
Доступность команды формы по условию Maks 07.04.2026 Алгоритм из решения ниже реализован на примере нетипового документа "СписаниеМатериалов", разработанного в конфигурации КА2. Задача: сделать доступной кнопку (команда формы "ЗавершитьСписание") при. . .	Уведомление о неверно выбранном значении справочника Maks 06.04.2026 Алгоритм из решения ниже реализован на примере нетипового документа "НарядПутевка", разработанного в конфигурации КА2. Задача: уведомлять пользователя, если в документе выбран неверный склад. . .	Установка Qt Creator для C и C++: ставим среду, CMake и MinGW без фреймворка Qt 8Observer8 05.04.2026 Среду разработки Qt Creator можно установить без фреймворка Qt. Есть отдельный репозиторий для этой среды: https:/ / github. com/ qt-creator/ qt-creator, где можно скачать установщик, на вкладке Releases:. . .	AkelPad-скрипты, структуры, и немного лирики.. testuser2 05.04.2026 Такая программа, как AkelPad существует уже давно, и также давно существуют скрипты под нее. Тем не менее, прога живет, периодически что-то не спеша дополняется, улучшается. Что меня в первую очередь. . .
Отображение реквизитов в документе по условию и контроль их заполнения Maks 04.04.2026 Алгоритм из решения ниже реализован на примере нетипового документа "ПланированиеСпецтехники", разработанного в конфигурации КА2. Данный документ берёт данные из другого нетипового документа. . .	Фото всей Земли с борта корабля Orion миссии Artemis II kumehtar 04.04.2026 Это первое подобное фото сделанное человеком за 50 лет. Снимок называют новым вариантом легендарной фотографии «The Blue Marble» 1972 года, сделанной с борта корабля «Аполлон-17». Новое фото. . .	Вывод диалогового окна перед закрытием, если документ не проведён Maks 04.04.2026 Алгоритм из решения ниже реализован на примере нетипового документа "СписаниеМатериалов", разработанного в конфигурации КА2. Задача: реализовать программный контроль на предмет проведения документа. . .	Программный контроль заполнения реквизитов табличной части документа Maks 02.04.2026 Алгоритм из решения ниже реализован на примере нетипового документа "СписаниеМатериалов", разработанного в конфигурации КА2. Задача: 1. Реализовать контроль заполнения реквизита. . .

@Almaz_1993 169 / 66 / 15 Регистрация: 24.03.2013 Сообщений: 467 Записей в блоге: 1

	Code review by Java Professional Developer 15.10.2015, 14:43. Показов 1181. Ответов 6 Метки нет (Все метки) Ребята, нужен человек с достаточно высоким левелом по Java, для проведения Code review моего проекта. Проект на гитхабе Симулятор торговли на фондовом рынке Начал этот проект недавно, но уже столкнулся с большими архитектурными проблемами. Поэтому прошу, у кого есть немного свободного времени проревьювить код и указать на недостатки и т.д. Заранее благодарен. 0

@KEKCoGEN 2399 / 2224 / 565 Регистрация: 28.12.2010 Сообщений: 8,672
	15.10.2015, 17:09
	Almaz_1993, для того чтобы делать код ревью для такого проекта нужно либо понимать в трейдинге, либо видеть диаграмму классов, их отношений, и flow диаграмму объясняющую что происходит в системе. Можно конечно долго читать код и все это понять, но мне нарпимер жаль на это времени. Из быстрого просмотра проекта: * Не храните lib на гитхабе. Мейвен должен сам приносить что надо * Проект использует (или не использует) некую библиотеку, которой нет в pom.xml. Импорты lt.monarch.* не резолвятся (я так понимаю это UI) * Все методы довольно длинные что затрудняет чтение и понимание кода. Метод не должен превышеть 10-50 строк * В программе используется константа DEFAULT_OUTPUT_REPORT_DIRECTORY которая указывает на директорию которой может и не быть. Эта настройка должна быть в проперти файлах или указывать на директорию проекта которая гарантировано существует. * BackTester tester = new BackTester(strategy, backTestDataProvider); -- зачем передавать провайдер если он есть как поле в стратегии? * Сам провайдер можно реализовать как итератор. Делать это через переменную currentBarIndex имхо так себе * В проекте указанно что для него необходима джава 8, однако никакого кода из джава 8 я не увидел (полагаю джава 8 это для javaFX, однако почему бы не писать уже код на джава 8 если она используется) Насчет архитектуры сказать немогу т.к не особо всматривался, но вроде ничего страшного не увидел. 1

@KEKCoGEN 2399 / 2224 / 565 Регистрация: 28.12.2010 Сообщений: 8,672
	15.10.2015, 19:45
	Almaz_1993, я бы не парился и так же написал бы. Можно впринципе придумать какого-нибудь монстра с рефлексией, но тогда этот код никто не поймет. 1

@Almaz_1993 169 / 66 / 15 Регистрация: 24.03.2013 Сообщений: 467 Записей в блоге: 1
	15.10.2015, 22:11 [ТС]
	Проблема еще в том, что не все проперти используются. Для одного репорт билдера - нужны одни данные, для другого другие. 0