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 **[ТС]**

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

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

Новые блоги и статьи Все статьи Все блоги /
Символьное дифференцирование igorrr37 13.02.2026 / * Логарифм записывается как: (x-2)log(x^2+2) - означает логарифм (x^2+2) по основанию (x-2). Унарный минус обозначается как ! в-строка - входное арифметическое выражение в инфиксной(обычной). . .	Камера Toupcam IUA500KMA Eddy_Em 12.02.2026 Т. к. у всяких "хикроботов" слишком уж мелкий пиксель, для подсмотра в ESPriF они вообще плохо годятся: уже 14 величину можно рассмотреть еле-еле лишь на экспозициях под 3 секунды (а то и больше),. . .	И ясному Солнцу zbw 12.02.2026 И ясному Солнцу, и светлой Луне. В мире покоя нет и люди не могут жить в тишине. А жить им немного лет.	«Знание-Сила» zbw 12.02.2026 «Знание-Сила» «Время-Деньги» «Деньги -Пуля»
SDL3 для Web (WebAssembly): Подключение Box2D v3, физика и отрисовка коллайдеров 8Observer8 12.02.2026 Содержание блога Box2D - это библиотека для 2D физики для анимаций и игр. С её помощью можно определять были ли коллизии между конкретными объектами и вызывать обработчики событий столкновения. . . .	SDL3 для Web (WebAssembly): Загрузка PNG с прозрачным фоном с помощью SDL_LoadPNG (без SDL3_image) 8Observer8 11.02.2026 Содержание блога Библиотека SDL3 содержит встроенные инструменты для базовой работы с изображениями - без использования библиотеки SDL3_image. Пошагово создадим проект для загрузки изображения. . .	SDL3 для Web (WebAssembly): Загрузка PNG с прозрачным фоном с помощью SDL3_image 8Observer8 10.02.2026 Содержание блога Библиотека SDL3_image содержит инструменты для расширенной работы с изображениями. Пошагово создадим проект для загрузки изображения формата PNG с альфа-каналом (с прозрачным. . .	Установка Qt-версии Lazarus IDE в Debian Trixie Xfce volvo 10.02.2026 В общем, достали меня глюки IDE Лазаруса, собранной с использованием набора виджетов Gtk2 (конкретно: если набирать текст в редакторе и вызвать подсказку через Ctrl+Space, то после закрытия окошка. . .

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

	Code review by Java Professional Developer 15.10.2015, 14:43. Показов 1163. Ответов 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

Опции темы