[SnowOne 2023] Валерия Юрковлянец: Как разработчику не потерять себя при работе на легаси-проекте

Что делать, если попал в легаси, но совсем не в Хогвартс?
Разберемся, как опознать легаси, откуда они берутся и самое главное — как приобрести релевантный опыт после работы на таком проекте

Transcript

1. Как разработчику не потерять себя на легаси проекте Лера Юрковлянец, CDEK IT
2. 2 @Slf4j @Service public class PodrygkaOrderStatusSendService { private final UpdateDeliveryStatusConnector connector; private final PodrygkaOrderStatusRepository statusRepository; public PodrygkaOrderStatusSendService( UpdateDeliveryStatusConnector connector, PodrygkaOrderStatusRepository statusRepository) { this.connector = connector; this.statusRepository = statusRepository; } public void sendOrderStatusList(List<PodrygkaOrderStatus> statuses) { if(CollectionUtils.isEmpty(statuses)){ return; } statuses.stream() .sorted(Comparator.comparing(PodrygkaOrderStatus::getCreated)) .forEach(status -> { //передаем терминальные статусы if (ORDER_ISSUED_TO_RECIPIENT.equals(status.getPodrygkaStatus()) || NOT_DELIVERED.equals(status.getCdekStatus())) { if (isOlderThenDay(status)) { sendAndValidateResponse(status); } //передаем промежуточные статусы } else if (isOlderThenHour(status)) { sendAndValidateResponse(status); } }); } }
3. 3 private void processColumns( boolean mainGraphs, GraphRequestData grd, Set<String> graphNames, Set<String> defaultGraphName, Map<LocalDate, EventStatsRow> periodToRowForAvgCalculation, EventStatsRow row, LocalDate today, CompanyGraphData resultRow, StatisticsResponseDto<EventStatsRow> statResult, LocalDate rowDate, GraphData calcData, BiFunction<LocalDate, Long, LocalDate> periodMinusFunc, Map<String, Integer> graphNameToPartialPeriods, int daysInPeriod, Set<String> otherGraphNamesToCheck, DateRangeOptionsDto dateRangeOptionsDto) { Set<PackKpiType> types = mainGraphs ? grd.getMainTypes() : grd.getOtherTypes(); for (PackType type : types) { switch (type) { case CPI -> processCpiColumn(grd, graphNames, defaultGraphName, row, today, resultRow, rowDate, calcData, periodMinusFunc, graphNameToPartialPeriods, kpiType); case RR -> processRrColumn(grd, graphNames, defaultGraphName, periodToRowForAvgCalculation, row, today, resultRow, statResult, rowDate, calcData, periodMinusFunc, graphNameToPartialPeriods, daysInPeriod, kpiType, appEventCohortDays, dateRangeOptionsDto); case ROAS -> processRoasColumn(grd, graphNames, defaultGraphName, periodToRowForAvgCalculation, row, today, resultRow, statResult, rowDate, calcData, periodMinusFunc, graphNameToPartialPeriods, daysInPeriod, kpiType, appEventCohortDays, dateRangeOptionsDto); case CVR -> processCvrColumn(grd, graphNames, defaultGraphName, periodToRowForAvgCalculation, row, today, resultRow, statResult, rowDate, calcData, periodMinusFunc, graphNameToPartialPeriods, daysInPeriod, kpiType, dynamicCohortDays, otherGraphNamesToCheck, dateRangeOptionsDto); case Cost -> processCostColumn(grd, graphNames, defaultGraphName, periodToRowForAvgCalculation, row, today, resultRow, statResult, rowDate, calcData, periodMinusFunc, graphNameToPartialPeriods, daysInPeriod, kpiType, dynamicCohortDays, otherGraphNamesToCheck, dateRangeOptionsDto); case CPA -> processCPAColumn(grd, graphNames, defaultGraphName, periodToRowForAvgCalculation, row, today, resultRow, statResult, rowDate, calcData, periodMinusFunc, graphNameToPartialPeriods, daysInPeriod, kpiType, dateRangeOptionsDto); } }
4. 4 150 строк
5. 5 700+ строк
6. Дисклеймер 6
7. Здесь и далее — реальный код, который крутится на продакшене и приносит деньги (большие) 7 Дисклеймер
8. Обо мне Java-разработчик в CDEK IT Блок интеграции НГТУ, РЭФ Адепт чистого кода и чистой архитектуры 8
9. Что по Легаси? 9
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. SOLID — принципы проектирования непосредственно программного кода 30 Всё до нас уже придумали
31. SOLID — принципы проектирования непосредственно программного кода Цель принципов — создать программные структуры, которые: • терпимы к изменениям • просты и понятны • образуют основу для компонентов, которые могут использоваться во многих программных системах 31 Всё до нас уже придумали
32. Немного о SOLID S — single responsibility principle / SRP O — open-closed principle / OCP L — Liskov substitution principle / LSP I — interface segregation principle / ISP D — dependency inversion principle / DIP 32
33. 33 protected CompanyService ( CompanyRepository repository , GDPRCountries gdprCountries , CreativeService creativeService , CountryService countryService , SSPService sspService , IABCategoryService iabCategoryService , BetAlgorithmService betAlgorithmService , BetAlgorithmLinkService betAlgorithmLinkService , @Lazy CompanyGlobalLimitService companyLimitService , AudienceService audienceService , AppEventService appEventService , @Lazy PackService packService , NamedParameterJdbcTemplate businessJdbcTemplate , NamedParameterJdbcTemplate statsJdbcTemplate , CompanyCriteriaRepository companyCriteriaRepository , SegmentService segmentService , PacingService pacingService , SettingsService settingsService , CreativeSelectionStrategyService creativeSelectionStrategyService , AutomaticModelTaskService automaticModelTaskService , CompanyBetPriceService companyBetPriceService , PmmlDataLoadRepository pmmlDataLoadRepository , CompanySspPackWhiteListService companySspPackWhiteListService , @Lazy SSPGroupService sspGroupService , SkanMappingService skanMappingService , RLAgentService rlAgentService , CompanyPredefinedTimeService companyPredefinedTimeService , GlobalModelInputSettingsService globalModelInputSettingsService , CompanyNotificationService companyNotificationService) { super (repository) ; this .gdprCountries = gdprCountries ; this .creativeService = creativeService ; this .countryService = countryService ; this .sspService = sspService ; this .iabCategoryService = iabCategoryService ; this .betAlgorithmService = betAlgorithmService ; this .betAlgorithmLinkService = betAlgorithmLinkService ; this .companyLimitService = companyLimitService ; this .audienceService = audienceService ; this .appEventService = appEventService ; this .packService = packService ; this .businessJdbcTemplate = businessJdbcTemplate ; this .statsJdbcTemplate = statsJdbcTemplate ; this .companyCriteriaRepository = companyCriteriaRepository ; this .segmentService = segmentService ; this .pacingService = pacingService ; this .settingsService = settingsService ; this .creativeSelectionStrategyService = creativeSelectionStrategyService ; this .automaticModelTaskService = automaticModelTaskService ; this .companyBetPriceService = companyBetPriceService ; this .pmmlDataLoadRepository = pmmlDataLoadRepository ; this .companySspPackWhiteListService = companySspPackWhiteListService ; this .sspGroupService = sspGroupService ; this .skanMappingService = skanMappingService ; this .rlAgentService = rlAgentService ; this .companyPredefinedTimeService = companyPredefinedTimeService ; this .globalModelInputSettingsService = globalModelInputSettingsService ; this .companyNotificationService = companyNotificationService ; } 25 зависимостей
34. SRP • Принцип единой ответственности 34
35. SRP • Принцип единой ответственности • У кода должна быть единственная цель и единственный актор 35
36. public class PaymentService { private final TerminalDao terminalDao ; private final AgentDao agentDao ; private final EmployeeService employeeService ; private final PostamatCashRegisterDao postamatCashRegisterDao ; private final OperationPaymentsRefundsDao operationPaymentsRefundsDao ; private final OperationDao operationDao ; private final CashNfcPayerService cashNfcPayerService ; private static final int TERMINAL_ACQUIRE_CODES_COUNT = 1; private Agent getPaymentAgentByCashboxSettings (CreationInput input) { if (input.getOrigin().getCashierShift() == null) { return getPaymentAgentByCashbox(input.getOrigin().getCash()) ; } return getPaymentAgentByCashRegisterId(input.getOrigin().getCashierShift().getCashRegister().getId()) ; } private Agent getPaymentAgentByCashboxSettings (EditionInput editInput) { if (editInput.getCashierShift() == null) { return getPaymentAgentByCashbox(editInput.getOldOperation().getCash()) ; } return getPaymentAgentByCashRegisterId(editInput.getCashierShift().getCashRegister().getId()) ; } @Nullable public Agent getPaymentAgent (CreationInput input) { if (input.getBaseOperationInfoForRefunds() != null || input.getType().isRefund()) { return getPaymentAgentFromRefund(input) ; } // … 36 Нормальный, на первый взгляд, класс
37. public class PaymentService { private final TerminalDao terminalDao ; private final AgentDao agentDao ; private final EmployeeService employeeService ; private final PostamatCashRegisterDao postamatCashRegisterDao ; private final OperationPaymentsRefundsDao operationPaymentsRefundsDao ; private final OperationDao operationDao ; private final CashNfcPayerService cashNfcPayerService ; private static final int TERMINAL_ACQUIRE_CODES_COUNT = 1; private Agent getPaymentAgentByCashboxSettings (CreationInput input) { if (input.getOrigin().getCashierShift() == null) { return getPaymentAgentByCashbox(input.getOrigin().getCash()) ; } return getPaymentAgentByCashRegisterId(input.getOrigin().getCashierShift().getCashRegister().getId()) ; } private Agent getPaymentAgentByCashboxSettings (EditionInput editInput) { if (editInput.getCashierShift() == null) { return getPaymentAgentByCashbox(editInput.getOldOperation().getCash()) ; } return getPaymentAgentByCashRegisterId(editInput.getCashierShift().getCashRegister().getId()) ; } @Nullable public Agent getPaymentAgent (CreationInput input) { if (input.getBaseOperationInfoForRefunds() != null || input.getType().isRefund()) { return getPaymentAgentFromRefund(input) ; } // … 37 Нормальный, на первый взгляд, класс
38. OSP • Принцип открытости-закрытости • Код должен быть открыт для расширения но закрыт для модификаций 38
39. 39 13 условных операторов
41. public class OperationEditPaymentService { private final TerminalDao terminalDao; private final AgentDao agentDao; private final PostamatCashRegisterDao postamatCashRegisterDao; private final OperationPaymentsRefundsDao operationPaymentsRefundsDao; private final OperationDao operationDao; private final CashNfcPayerService cashNfcPayerService; private static final int TERMINAL_ACQUIRE_CODES_COUNT = 1; @Nullable public Agent getPaymentAgent(EditionInput editInput) { if (editInput.getOldOperation().getType().isRefund()) { return getPaymentAgentFromRefund(editInput); } if (isQrCodePayment(editInput)) { return getPaymentAgentByUuid(editInput.getSbpMerchant().getAgentUuid()); } if (editInput.getCardPayment() != null && CardPaymentSystemType.isNfc(editInput.getCardPayment().getPaymentSystem())) { return getNfcPaymentAgentByCashbox(editInput.getCash()); } switch (editInput.getOldOperation().getSourceType()){ case OFFICE: case COURIER: if (editInput.getCourierCode() == null) { CourierRevise courierRevise = editInput.getOldOperation().getCourierRevise(); if (courierRevise == null) { return getOfficePaymentAgent(editInput); } else { return getPaymentAgentByCourier(courierRevise.getCourierCode()); ... 41
42. 42 public class OperationCreatePaymentService { private final TerminalDao terminalDao; private final AgentDao agentDao; private final EmployeeService employeeService; private final PostamatCashRegisterDao postamatCashRegisterDao; private final CashNfcPayerService cashNfcPayerService; private static final int TERMINAL_ACQUIRE_CODES_COUNT = 1; @Nullable public Agent getPaymentAgent(CreationInput input) { if (input.getBaseOperationInfoForRefunds() != null || input.getType().isRefund()) { return getPaymentAgentFromRefund(input); } if (isQrCodePayment(input)) { return getPaymentAgentByUuid(input.getSbpMerchant().getAgentUuid()); } if (input.getCardPaymentInfo() != null && CardPaymentSystemType.isNfc(input.getCardPaymentInfo().getPaymentSystemType())) { return getNfcPaymentAgentByCashbox(input.getOrigin().getCash()); } switch (input.getOperationSourceType()) { case OFFICE: return getPaymentAgentByCashboxSettings(input); case SITE: return getPaymentAgentByPaymentSystem(input.getCardPaymentInfo().getPaymentSystemType()); case COURIER: return getPaymentAgentByMobileCourier((MobileCourier) input.getSourceDocument(), input.getOrigin().getCash()); ...
43. 43 public class OperationCreatePaymentService { private final TerminalDao terminalDao; private final AgentDao agentDao; private final EmployeeService employeeService; private final PostamatCashRegisterDao postamatCashRegisterDao; private final CashNfcPayerService cashNfcPayerService; private static final int TERMINAL_ACQUIRE_CODES_COUNT = 1; @Nullable public Agent getPaymentAgent(CreationInput input) { if (input.getBaseOperationInfoForRefunds() != null || input.getType().isRefund()) { return getPaymentAgentFromRefund(input); } if (isQrCodePayment(input)) { return getPaymentAgentByUuid(input.getSbpMerchant().getAgentUuid()); } if (input.getCardPaymentInfo() != null && CardPaymentSystemType.isNfc(input.getCardPaymentInfo().getPaymentSystemType())) { return getNfcPaymentAgentByCashbox(input.getOrigin().getCash()); } switch (input.getOperationSourceType()) { case OFFICE: return getPaymentAgentByCashboxSettings(input); case SITE: return getPaymentAgentByPaymentSystem(input.getCardPaymentInfo().getPaymentSystemType()); case COURIER: return getPaymentAgentByMobileCourier((MobileCourier) input.getSourceDocument(), input.getOrigin().getCash()); ... switch (input.getOperationSourceType()) { case OFFICE: return getPaymentAgentByCashboxSettings(input); case SITE: return getPaymentAgentByPaymentSystem(input.getCardPaymentInfo().getPaymentSystemType()); case COURIER: UUID sourceUuid = input.getSourceDocument().getSourceUuid(); String courierCode = ((CourierDelivery) input.getSourceDocument()).getCourierCode(); return getPaymentAgentByMobileCourier(courierCode, sourceUuid, input.getOrigin().getCash(); default: return null; }
44. 44 public class OperationCreatePaymentService { private final TerminalDao terminalDao; private final AgentDao agentDao; private final EmployeeService employeeService; private final PostamatCashRegisterDao postamatCashRegisterDao; private final CashNfcPayerService cashNfcPayerService; private static final int TERMINAL_ACQUIRE_CODES_COUNT = 1; @Nullable public Agent getPaymentAgent(CreationInput input) { // todo в оффлайн режиме не проставляется базовая операция, но и в isRefund нет нефискальных БО // будет исправлено в CASH-1512 if (input.getBaseOperationInfoForRefunds() != null || input.getType().isRefund()) { return getPaymentAgentFromRefund(input); } // Оплата через СБП (с помощью QR кода) может быть как с OFFICE с МК или Сайта if (isQrCodePayment(input)) { return getPaymentAgentByUuid(input.getSbpMerchant().getAgentUuid()); } // Если оплата через NFC то подтягиваем связанного с кассой NFC эквайера (может быть как с МК так и с OFFICE) if (input.getCardPaymentInfo() != null && CardPaymentSystemType.isNfc(input.getCardPaymentInfo().getPaymentSystemType())) { return getNfcPaymentAgentByCashbox(input.getOrigin().getCash()); } switch (input.getOperationSourceType()) { case OFFICE: return getPaymentAgentByCashboxSettings(input); case SITE: return getPaymentAgentByPaymentSystem(input.getCardPaymentInfo().getPaymentSystemType()); case COURIER: return getPaymentAgentByMobileCourier((MobileCourier) input.getSourceDocument(), input.getOrigin().getCash()); ... switch (input.getOperationSourceType()) { case OFFICE: return getPaymentAgentByCashboxSettings(input); case SITE: return getPaymentAgentByPaymentSystem(input.getCardPaymentInfo().getPaymentSystemType()); case COURIER: UUID sourceUuid = input.getSourceDocument().getSourceUuid(); String courierCode = ((CourierDelivery) input.getSourceDocument()).getCourierCode(); return getPaymentAgentByMobileCourier(courierCode, sourceUuid, input.getOrigin().getCash(), input.getOrigin().getLang()); case POSTAMAT: return getPaymentAgentByPostamatCashRegister( ((PostomatPaymentRequest) input.getSourceDocument()).getOfficeUuid()); case EVENT_DELIVERY_DETAIL: return getPaymentAgentByCashbox( (input.getOrigin().getCash())); case PARTNER: return getPaymentAgent(input); default: return null; }
45. 45 public class CourierOperationCreatePaymentService implements OperationCreatePaymentService{ private final TerminalDao terminalDao; private final AgentService agentService; private final EmployeeService employeeService; private static final int TERMINAL_ACQUIRE_CODES_COUNT = 1; @Nullable public Agent getPaymentAgent(CreationInput input) { UUID sourceUuid = input.getSourceDocument().getSourceUuid(); String courierCode = ((CourierDelivery) input.getSourceDocument()).getCourierCode(); return getPaymentAgentByMobileCourier(courierCode, sourceUuid, input.getOrigin().getCash(), input.getOrigin().getLang()); } private Agent getPaymentAgentByMobileCourier(String courierCode, UUID sourceUuid, Cash cash, String lang) { Employee courier = employeeService.getEmployee(courierCode, lang) .orElseThrow(() -> new CdekReferenceException(ErrorType.COURIER_NOT_FOUND, "Courier not found for code ", courierCode)); List<UUID> payerUuids = terminalDao.getPayerUuidsByCourierCode(courier.getCode()); if (!courier.isActive() && payerUuids.isEmpty()) { return getPaymentAgentByCashbox(cash); } if (payerUuids.size() > TERMINAL_ACQUIRE_CODES_COUNT) { throw new CdekCashboxException( ErrorType.MORE_THAN_ONE_TERMINAL_COURIER, "There are more than one relation with the terminal and courier with id = " + courierCode, sourceUuid, courierCode ); } ...
46. 46 public class CourierOperationCreatePaymentService implements OperationCreatePaymentService{ private final TerminalDao terminalDao; private final AgentService agentService; private final EmployeeService employeeService; private static final int TERMINAL_ACQUIRE_CODES_COUNT = 1; @Nullable public Agent getPaymentAgent(CreationInput input) { UUID sourceUuid = input.getSourceDocument().getSourceUuid(); String courierCode = ((CourierDelivery) input.getSourceDocument()).getCourierCode(); return getPaymentAgentByMobileCourier(courierCode, sourceUuid, input.getOrigin().getCash(), input.getOrigin().getLang()); } private Agent getPaymentAgentByMobileCourier(String courierCode, UUID sourceUuid, Cash cash, String lang) { Employee courier = employeeService.getEmployee(courierCode, lang) .orElseThrow(() -> new CdekReferenceException(ErrorType.COURIER_NOT_FOUND, "Courier not found for code ", courierCode)); List<UUID> payerUuids = terminalDao.getPayerUuidsByCourierCode(courier.getCode()); if (!courier.isActive() && payerUuids.isEmpty()) { return getPaymentAgentByCashbox(cash); } if (payerUuids.size() > TERMINAL_ACQUIRE_CODES_COUNT) { throw new CdekCashboxException( ErrorType.MORE_THAN_ONE_TERMINAL_COURIER, "There are more than one relation with the terminal and courier with id = " + courierCode, sourceUuid, courierCode ); } ... public interface OperationCreatePaymentService { Agent getPaymentAgent(CreationInput input); }
47. 47 class CourierOperationCreatePaymentService class SiteOperationCreatePaymentService class PostamatOperationCreatePaymentService class OfficeOperationCreatePaymentService class PartnerOperationCreatePaymentService
48. 48 class CourierOperationCreatePaymentService class SiteOperationCreatePaymentService class PostamatOperationCreatePaymentService class OfficeOperationCreatePaymentService class PartnerOperationCreatePaymentService public interface PaymentServiceOperationSourceFactory { /** * Получить источник сервис Плательщика * по типу операции * @param type Тип источника операции * @return сервис */ PaymentService get(OperationSourceType type); }
50. ISP • Принцип разделения интерфейсов • Объекты не должны зависеть от интерфейсов, которые они не используют 50
51. public class CreationInput { private final Origin origin; private OperationType type; private OperationSubtype operationSubtype; private String cashFlowItemNumber; private Instant creationDate; private LocalDate date; private final UnderlyingDocument underlyingDocument; @Nullable private final Agent agent; @Nullable private Individual individual; private Contract contract; private final Person person; private final String description; private ReceiptInfo receiptInfo; private ReceiptData receiptData; private final CompositePayment paid; private OperationSourceType operationSourceType; private OperationSource sourceDocument; private boolean fromDraft; private Cash destinationCashbox; private CardPaymentInfo cardPaymentInfo; private LocalDate cashboxPeriodClosingMoment; 51
52. public class CreationInput { private final Origin origin; private OperationType type; private OperationSubtype operationSubtype; private String cashFlowItemNumber; private Instant creationDate; private LocalDate date; private final UnderlyingDocument underlyingDocument; @Nullable private final Agent agent; @Nullable private Individual individual; private Contract contract; private final Person person; private final String description; private ReceiptInfo receiptInfo; private ReceiptData receiptData; private final CompositePayment paid; private OperationSourceType operationSourceType; private OperationSource sourceDocument; private boolean fromDraft; private Cash destinationCashbox; private CardPaymentInfo cardPaymentInfo; private LocalDate cashboxPeriodClosingMoment; 52 30 полей в сущности
53. public class CreationInput { private final Origin origin; private OperationType type; private OperationSubtype operationSubtype; private String cashFlowItemNumber; private Instant creationDate; private LocalDate date; private final UnderlyingDocument underlyingDocument; @Nullable private final Agent agent; @Nullable private Individual individual; private Contract contract; private final Person person; private final String description; private ReceiptInfo receiptInfo; private ReceiptData receiptData; private final CompositePayment paid; private OperationSourceType operationSourceType; private OperationSource sourceDocument; private boolean fromDraft; private Cash destinationCashbox; private CardPaymentInfo cardPaymentInfo; private LocalDate cashboxPeriodClosingMoment; private SbpMerchant sbpMerchant; private String qrSourceId; private String courierCode; @Nullable private PointsPaymentData pointsPaymentData; private List<Operation> previous; private boolean hasSiteOperation; private CashAvailablePeriod cashAvailablePeriod; @Nullable private LegalEntity merchantLegalEntity; @Nullable private UUID cashQrPayerUuid; } 53 30 полей в сущности
54. 54 public class CourierOperationCreatePaymentService implements OperationCreatePaymentService{ private final TerminalDao terminalDao; private final AgentService agentService; private final EmployeeService employeeService; private static final int TERMINAL_ACQUIRE_CODES_COUNT = 1; @Nullable public Agent getPaymentAgent(OperationPayerContext input) { UUID sourceUuid = input.getSourceDocument().getSourceUuid(); String courierCode = ((CourierDelivery) input.getSourceDocument()).getCourierCode(); return getPaymentAgentByMobileCourier(courierCode, sourceUuid, input.getOrigin().getCash(), input.getOrigin().getLang()); } private Agent getPaymentAgentByMobileCourier(String courierCode, UUID sourceUuid, Cash cash, String lang) { Employee courier = employeeService.getEmployee(courierCode, lang) .orElseThrow(() -> new CdekReferenceException(ErrorType.COURIER_NOT_FOUND, "Courier not found for code ", courierCode)); List<UUID> payerUuids = terminalDao.getPayerUuidsByCourierCode(courier.getCode()); if (!courier.isActive() && payerUuids.isEmpty()) { return getPaymentAgentByCashbox(cash); } if (payerUuids.size() > TERMINAL_ACQUIRE_CODES_COUNT) { throw new CdekCashboxException( ErrorType.MORE_THAN_ONE_TERMINAL_COURIER, "There are more than one relation with the terminal and courier with id = " + courierCode, sourceUuid, courierCode ); } ... public interface OperationPayerContext { OperationSource getSourceDocument(); Origin getOrigin(); UUID getCashQrPayerUuid(); CardPaymentInfo getCardPaymentInfo(); }
55. 55 public class CourierOperationCreatePaymentService implements OperationCreatePaymentService{ private final TerminalDao terminalDao; private final AgentService agentService; private final EmployeeService employeeService; private static final int TERMINAL_ACQUIRE_CODES_COUNT = 1; @Nullable public Agent getPaymentAgent(OperationPayerContext input) { UUID sourceUuid = input.getSourceDocument().getSourceUuid(); String courierCode = ((CourierDelivery) input.getSourceDocument()).getCourierCode(); return getPaymentAgentByMobileCourier(courierCode, sourceUuid, input.getOrigin().getCash(), input.getOrigin().getLang()); } private Agent getPaymentAgentByMobileCourier(String courierCode, UUID sourceUuid, Cash cash, String lang) { Employee courier = employeeService.getEmployee(courierCode, lang) .orElseThrow(() -> new CdekReferenceException(ErrorType.COURIER_NOT_FOUND, "Courier not found for code ", courierCode)); List<UUID> payerUuids = terminalDao.getPayerUuidsByCourierCode(courier.getCode()); if (!courier.isActive() && payerUuids.isEmpty()) { return getPaymentAgentByCashbox(cash); } if (payerUuids.size() > TERMINAL_ACQUIRE_CODES_COUNT) { throw new CdekCashboxException( ErrorType.MORE_THAN_ONE_TERMINAL_COURIER, "There are more than one relation with the terminal and courier with id = " + courierCode, sourceUuid, courierCode ); } ... public interface OperationPayerContext { OperationSource getSourceDocument(); Origin getOrigin(); UUID getCashQrPayerUuid(); CardPaymentInfo getCardPaymentInfo(); }
56. 56 Распространенные ошибки
57. • Длинные и неразборчивые типы • Хардкодинг и магические числа • Недостаточное ревью 57 Распространенные ошибки
58. Длинные неразборчивые типы public class EntityGraphCounterDTO { Map<LocalDateTime, Map<CounterType, Number>> dateToCounters; Map<LocalDateTime, Set<GraphChangesType>> changes; } 58 Map<Long, Long> maxChangelogIdByCid = new HashMap<>(); Map<Long, Pair<LocalDateTime, List<GraphChangesType>>> changelogIdToData = new HashMap<>(); if (CollectionUtils.isNotEmpty(lastCompanyChanges)) { for (int i = 0; i < lastCompanyChanges.size() - 1; i++) { final ChangeLog currentChange = lastCompanyChanges.get(i); final ChangeLog prevChange = lastCompanyChanges.get(i + 1); CompanyDto currentCompany; CompanyDto prevCompany; try { currentCompany = OBJECT_MAPPER.readValue(currentChange.getContent(), CompanyDto.class); prevCompany = OBJECT_MAPPER.readValue(prevChange.getContent(), CompanyDto.class); } catch (Exception e) { throw new UIException("changelog.parse.error", HttpStatus.INTERNAL_SERVER_ERROR); } if (!Objects.equals(currentCompany.getId(), prevCompany.getId())) { // we shouldn't check changes between different companies continue; }
59. Длинные неразборчивые типы public class EntityGraphCounterDTO { Map<LocalDateTime, Map<CounterType, Number>> dateToCounters; Map<LocalDateTime, Set<GraphChangesType>> changes; } 59 Map<Long, Long> maxChangelogIdByCid = new HashMap<>(); Map<Long, Pair<LocalDateTime, List<GraphChangesType>>> changelogIdToData = new HashMap<>(); if (CollectionUtils.isNotEmpty(lastCompanyChanges)) { for (int i = 0; i < lastCompanyChanges.size() - 1; i++) { final ChangeLog currentChange = lastCompanyChanges.get(i); final ChangeLog prevChange = lastCompanyChanges.get(i + 1); CompanyDto currentCompany; CompanyDto prevCompany; try { currentCompany = OBJECT_MAPPER.readValue(currentChange.getContent(), CompanyDto.class); prevCompany = OBJECT_MAPPER.readValue(prevChange.getContent(), CompanyDto.class); } catch (Exception e) { throw new UIException("changelog.parse.error", HttpStatus.INTERNAL_SERVER_ERROR); } if (!Objects.equals(currentCompany.getId(), prevCompany.getId())) { // we shouldn't check changes between different companies continue; }
60. Хардкодинг if (dto.isOverridePackUrls()) { checkTrue(StringUtils.isNotBlank(dto.getClickURL()), "company.custom.url.required", HttpStatus.BAD_REQUEST, "click"); checkTrue(StringUtils.isNotBlank(dto.getDestinationURL()), "company.custom.url.required", HttpStatus.BAD_REQUEST, "destination"); if (!skipAdvertiserIdUrlCheck) { checkTrue(UIDtosUtil.validateAdvertiserIdURL(dto.getClickURL(), isGdprCountry), isGdprCountry ? "company.url.gdpr.invalid" : "company.url.non-gdpr.invalid", HttpStatus.BAD_REQUEST); } if (StringUtils.isNotBlank(dto.getImpressionURL())) { if (!skipAdvertiserIdUrlCheck) { checkTrue(UIDtosUtil.validateAdvertiserIdURL(dto.getImpressionURL(), isGdprCountry), isGdprCountry ? "company.url.gdpr.invalid" : "company.url.non-gdpr.invalid", HttpStatus.BAD_REQUEST); } checkNotNull(dto.getImpressionVideoEventTrigger(), "impression.video-event-trigger-required", HttpStatus.BAD_REQUEST); } CreativeService.validateUrlEncodeTags(dto.getDestinationURL(), "Destination"); CreativeService.validateUrlEncodeTags(dto.getClickURL(), "Click"); CreativeService.validateUrlEncodeTags(dto.getImpressionURL(), "Impression"); } else { if (!skipAdvertiserIdUrlCheck) { Pack pack = packService.findByName(dto.getPackId()); checkTrue(UIDtosUtil.validateAdvertiserIdURL(pack.getClickURL(), isGdprCountry), isGdprCountry ? "pack.url.gdpr.invalid" : "pack.url.non-gdpr.invalid", HttpStatus.BAD_REQUEST); checkTrue(pack.getServerClickURL() == null || UIDtosUtil.validateAdvertiserIdURL(pack.getServerClickURL(), isGdprCountry), isGdprCountry ? "pack.url.gdpr.invalid" : "pack.url.non-gdpr.invalid", HttpStatus.BAD_REQUEST); checkTrue(pack.getImpressionURL() == null || UIDtosUtil.validateAdvertiserIdURL(pack.getImpressionURL(), isGdprCountry), isGdprCountry ? "pack.url.gdpr.invalid" : "pack.url.non-gdpr.invalid", HttpStatus.BAD_REQUEST); } } 60
61. Хардкодинг if (dto.isOverridePackUrls()) { checkTrue(StringUtils.isNotBlank(dto.getClickURL()), "company.custom.url.required", HttpStatus.BAD_REQUEST, "click"); checkTrue(StringUtils.isNotBlank(dto.getDestinationURL()), "company.custom.url.required", HttpStatus.BAD_REQUEST, "destination"); if (!skipAdvertiserIdUrlCheck) { checkTrue(UIDtosUtil.validateAdvertiserIdURL(dto.getClickURL(), isGdprCountry), isGdprCountry ? "company.url.gdpr.invalid" : "company.url.non-gdpr.invalid", HttpStatus.BAD_REQUEST); } if (StringUtils.isNotBlank(dto.getImpressionURL())) { if (!skipAdvertiserIdUrlCheck) { checkTrue(UIDtosUtil.validateAdvertiserIdURL(dto.getImpressionURL(), isGdprCountry), isGdprCountry ? "company.url.gdpr.invalid" : "company.url.non-gdpr.invalid", HttpStatus.BAD_REQUEST); } checkNotNull(dto.getImpressionVideoEventTrigger(), "impression.video-event-trigger-required", HttpStatus.BAD_REQUEST); } CreativeService.validateUrlEncodeTags(dto.getDestinationURL(), "Destination"); CreativeService.validateUrlEncodeTags(dto.getClickURL(), "Click"); CreativeService.validateUrlEncodeTags(dto.getImpressionURL(), "Impression"); } else { if (!skipAdvertiserIdUrlCheck) { Pack pack = packService.findByName(dto.getPackId()); checkTrue(UIDtosUtil.validateAdvertiserIdURL(pack.getClickURL(), isGdprCountry), isGdprCountry ? "pack.url.gdpr.invalid" : "pack.url.non-gdpr.invalid", HttpStatus.BAD_REQUEST); checkTrue(pack.getServerClickURL() == null || UIDtosUtil.validateAdvertiserIdURL(pack.getServerClickURL(), isGdprCountry), isGdprCountry ? "pack.url.gdpr.invalid" : "pack.url.non-gdpr.invalid", HttpStatus.BAD_REQUEST); checkTrue(pack.getImpressionURL() == null || UIDtosUtil.validateAdvertiserIdURL(pack.getImpressionURL(), isGdprCountry), isGdprCountry ? "pack.url.gdpr.invalid" : "pack.url.non-gdpr.invalid", HttpStatus.BAD_REQUEST); } } 61
62. Недостаточное ревью private List<GraphChangesType> detectRequiredPaymentChanges(PaymentDTO currentPayment, PaymentDto previousPayment) { List<GraphChangesType> result = new ArrayList<>(); if (!Objects.equals(currentCreative.getMinCPM(), previousCreative.getMinCPM()) || !Objects.equals(currentCreative.getMaxCPM(), currentCreative.getMaxCPM())) { result.add(GraphChangesType.CreativeBet); } return result; } 62
63. Недостаточное ревью private List<GraphChangesType> detectRequiredPaymentChanges(PaymentDTO currentPayment, PaymentDto previousPayment) { List<GraphChangesType> result = new ArrayList<>(); if (!Objects.equals(currentCreative.getMinCPM(), previousCreative.getMinCPM()) || !Objects.equals(currentCreative.getMaxCPM(), currentCreative.getMaxCPM())) { result.add(GraphChangesType.CreativeBet); } return result; } 63
64. 64 Почему легаси — плохо
65. • Разработка дольше 65 Почему легаси — плохо
66. • Разработка дольше • Код бажный и непредсказуемый 66 Почему легаси — плохо
67. • Разработка дольше • Код бажный и непредсказуемый • Падают скилы 67 Почему легаси — плохо
68. Почему легаси — плохо • Разработка дольше • Код бажный и непредсказуемый • Падают скилы 68
70. 70 Что делать
71. • Писать тесты 71 Что делать
72. • Писать тесты • Анализировать код 72 Что делать
73. • Писать тесты • Анализировать код 73 Что делать
74. • Писать тесты • Анализировать код • Применять правило бой-скаутов 74 Что делать
75. • Писать тесты • Анализировать код • Применять правило бой-скаутов • Держать в голове стандарты кода 75 Что делать
76. Что делать • Писать тесты • Анализировать код • Применять правило бой-скаутов • Держать в голове стандарты кода • Задавать вопросы 76
77. Что ещё почитать 77
78. Что ещё почитать • принципы проектирования KISS — DRY — YAGNI — 78
79. • принципы проектирования KISS — Keep It Simple, Stupid DRY — YAGNI — 79 Что ещё почитать
80. • принципы проектирования KISS — Keep It Simple, Stupid DRY — Don’t Repeat Yourself YAGNI — 80 Что ещё почитать
81. • принципы проектирования KISS — Keep It Simple, Stupid DRY — Don’t Repeat Yourself YAGNI — You Aren’t Gonna Need It 81 Что ещё почитать
82. • принципы проектирования KISS — Keep It Simple, Stupid DRY — Don’t Repeat Yourself YAGNI — You Aren’t Gonna Need It • Чистый код и Чистая архитектура 82 Что ещё почитать
83. Спасибо! Вопросы 83 Лера Юрковлянец @yura_kovlyanets