Skip to content

smells (online team) #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
NoticeMeCapcake wants to merge 1 commit into
base: main
Choose a base branch
from
Open

smells (online team) #4

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/main/java/ru/cu/OrderProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,15 @@
import java.util.logging.Level;
import java.util.logging.Logger;

// я бы это вынес в отдельный файл
abstract class BaseOrderProcessor {
public void beforeProcess() {}
public void afterProcess() {}
}

// очень забавное использование Closeable -- туда же Override

// Class очень большой
public class OrderProcessor extends BaseOrderProcessor implements Closeable {

public static final OrderProcessor INSTANCE = new OrderProcessor();
Expand All @@ -30,6 +34,7 @@ public class OrderProcessor extends BaseOrderProcessor implements Closeable {
private double lastGrandTotal;
private int operationsCounter;

// Ещё один функционал, который излишен для этого класса
private final Map<String, Double> loyaltyCache = new HashMap<>();

private String tempReport;
Expand Down Expand Up @@ -73,6 +78,7 @@ public void processOrders(List<Order> orders,
}
}

// Считаем что-то с деньгами в даблах
double shippingCost = calculateShipping(shippingMethod, shippingSpeed);
double tax = total * taxRate;
double grandTotal = total + shippingCost + tax;
Expand All @@ -97,6 +103,7 @@ public void processOrders(List<Order> orders,
}

tempReport = generateReport(customerName, orders, grandTotal);
// сомнительно, зачем fine
logger.fine(tempReport);

lastCustomerName = customerName;
Expand All @@ -105,6 +112,7 @@ public void processOrders(List<Order> orders,
afterProcess();
}

// Непонятный switch с непонятными значениями и магическими числами
private double calculateShipping(int method, int speed) {
switch (method) {
case 1 -> {
Expand All @@ -119,6 +127,7 @@ private double calculateShipping(int method, int speed) {
}
}

// Вынести бы в отдельную сущность и класс для генерации репорта
@Deprecated
public String generateReport(String customerName, List<Order> orders, double grandTotal) {
StringBuilder sb = new StringBuilder();
Expand All @@ -133,13 +142,16 @@ public String generateReport(String customerName, List<Order> orders, double gra
return sb.toString();
}

// не юзается
public String getCustomerPhone(Order o) {
return o.getCustomer().getPhone();
}
// не юзается
public String getCustomerEmail(Order o) {
return o.getCustomer().getEmail();
}

// слишком много функционала
private void sendEmail(String name, String a1, String a2, String pc, String city, String country, double total) {
try {
if (total < 0) throw new IOException("Negative total!");
Expand All @@ -148,10 +160,12 @@ private void sendEmail(String name, String a1, String a2, String pc, String city
}
}

// слишком много функционала
private void sendSMS(String name, String country, double total) {
logger.info("[SMS] Hello, " + name + " from " + country + ": " + total);
}

// слишком много функционала
private void sendPush(String name, double total) {
logger.info("[PUSH] Hi " + name + ": " + total);
}
Expand All @@ -160,6 +174,7 @@ private void logTransaction(String customer, double total) {
logger.log(Level.INFO, customer + " spent " + total);
}

// не юзается
public double calculateTotalAmounts(List<Order> orders) {
double sum = 0;
for (Order o : orders) {
Expand All @@ -168,6 +183,7 @@ public double calculateTotalAmounts(List<Order> orders) {
return sum;
}

// не юзается + ВРЗВРАЩАЕТ 42?)))
private double oldCalculate(List<Order> orders) {
return 42;
}
Expand All @@ -176,6 +192,7 @@ private double oldCalculate(List<Order> orders) {
public void close() {
}

// надо вынести
public static class Order {
private final String id;
private final double amount;
Expand All @@ -191,6 +208,7 @@ public Order(String id, double amount, Customer customer) {
public Customer getCustomer() { return customer; }
}

// надо вынести
public static class Customer {
private final String name;
private final String phone;
Expand All @@ -205,6 +223,7 @@ public Customer(String name, String phone, String email, Address address) {
public Address getAddress() { return address; }
}

// надо вынести
public static class Address {
private final String line1;
private final String line2;
Expand Down