1 Востаннє редагувалося Lokki (19.05.2016 17:48:10)

Тема: Оцінка коду

Добрий день

Отримав невеликве завдання по Джаві, раніше програмував тільки на С++.
Потрібен погляд зі сторони. Мені здається що виглядає програма не дуже ну зі сторони ОПП.
Можете прокоментувати.

Завдання: Implement a simple point of sale.

Assume you have:
– one input device: bar codes scanner
– two output devices: LCD display and printer

Implement:
– single product sale: products bar code is scanned and:
– if the product is found in products database than it's name and price is printed on LCD
– if the product is not found than error message 'Product not found' is printed on LCD
– if the code scanned is empty than error message 'Invalid bar-code' is printed on LCD
– when 'exit' is input than receipt is printed on printer containing a list of all previously scanned items names and prices as well as total sum to be paid for all items; the total sum is also printed on LCD display

Rules:
– use only SDK classes and your favorite test libraries
– mock/stub the database and IO devices
– concentrate on proper design and clean code, rather than supplying fully functioning application

Реалізація:
Клас SalePoint

public class SalePoint {
    private InputDevice codesScanner = new CodesScanner();
    private DBWorker dbWorker = null;
    private OutputDevice lcdDisplay = new LCDDisplay();
    private OutputDevice printer = new Printer();

    private List<Product> list = new ArrayList<Product>();

    public SalePoint(){
        try {
            dbWorker = new DBWorker();
        } catch (SQLException e) {
            e.printStackTrace();
        }
    }

    public void setDefaultBarCodes(){
        codesScanner.write("exit");
        codesScanner.write("");
        codesScanner.write("0000000001");
        codesScanner.write("0000000002");
    }


    public void run() {
        try {
            Product product = null;
            while(codesScanner.hasNext()) {
                String barCode = codesScanner.readLine();

                switch (barCode) {
                    case "":
                        lcdDisplay.printError("Product not found");
                        break;
                    case "exit":
                        printer.println("Printer: ");
                        printProducts();
                        return;
                    default:
                        product = dbWorker.findProduct(barCode);

                        if (product == null) {
                            lcdDisplay.printError("Invalid bar-code");
                        }

                        lcdDisplay.print(product);
                        list.add(product);
                }
            }

        } catch (EmptyStackException e) {
            e.printStackTrace();
        }
    }

    private void printProducts() {
        double summ = 0.0;

        for (Product product :
                list) {
            summ += product.getPrice();
        }

        ((Printer)printer).printProductsAndTotalPrice(list, summ);

    }
}

Class InputDevice

public class InputDevice {
    private Stack<String> buffer = new Stack<>();

    public void write(String message){
        buffer.push(message);
    }

    public String readLine(){
        if(buffer.isEmpty())
            return null;
        return buffer.pop();
    }

    public void clear(){
        buffer.clear();
    }

    public boolean hasNext(){
        return !buffer.isEmpty();
    }
}

Class CodesScaner

public class CodesScanner extends InputDevice{

    public CodesScanner() {
        super();
    }
// Не знаю який метод додати. 

}

Class OutputDevice

public class OutputDevice {
    void println(String message){
        System.out.println(message);
    }
    void print(String message){
        System.out.print(message);
    }
    void printError(String message){
        System.out.println("Error: " + message);
    }
    void print(Product product){
        System.out.println(product);
    }
}

Class Printer

public class Printer extends OutputDevice {
    public Printer() {
        super();
    }

    public void printProducts(List<Product> productList){
        System.out.println("List of products:");
        for (Product product:
             productList) {
            System.out.println(product.getName() + "\t\t\t" + product.getPrice());
        }
    }

    public void printProductsAndTotalPrice(List<Product> productList, double totalPrice){
        System.out.println("List of products:\nName\t\t\tPrice");
        for (Product product:
                productList) {
            System.out.println(product.getName() + "\t\t\t" + product.getPrice());
        }
        System.out.println("Total:\t\t\t" + totalPrice);
    }

}

Class LCDDisplay

public class LCDDisplay extends OutputDevice {
    public LCDDisplay() {
    }

    public void print(Product product){
        System.out.println(product.getName() + "\t\t" + product.getPrice());
    }

}

Class DBWorker

public class DBWorker {
    private static String URL = "jdbc:mysql://localhost:3306/mydbtest";
    private static String USERNAME = "root";
    private static String PASSWORD = "root";
    private static String SELECT = "SELECT product_name, price, bar_code FROM products WHERE bar_code = ?;";

    private Connection connection;

    public DBWorker() throws SQLException {
        Driver driver = new FabricMySQLDriver();
        DriverManager.registerDriver(driver);

        connection = DriverManager.getConnection(URL, USERNAME, PASSWORD);
    }

    public Product findProduct(String barCode) {
        Product product = null;

        PreparedStatement preparedStatement = null;
        try {
            preparedStatement = connection.prepareStatement(SELECT);

            preparedStatement.setString(1, barCode);

            ResultSet resultSet = preparedStatement.executeQuery();

            if(resultSet.next())
                product = new Product(resultSet.getString("product_name"), resultSet.getInt("price"), resultSet.getString("bar_code"));

        } catch (SQLException e) {
            e.printStackTrace();
        }

        return product;
    }
}

main

public class Main {
    public static void main(String[] args) {
        SalePoint salePoint = new SalePoint();

        salePoint.setDefaultBarCodes();
        salePoint.run();

    }
}

Як на ваш погляд моя реалізація?

І не підскажите як зробити тести? Juint i Mockito

Тільки один зробив але не розумію навіщо він - бесполезний.

public class DBTest {
    private static final Product PRODUCT = new Product("beer", 2.3, "0000000001");
    private static final String BAR_CODE = "0000000001";

    @Mock
    SalePoint salePoint;

    @Test
    public void testFindProduct(){
        DBWorker dbWorker = mock(DBWorker.class);

        when(dbWorker.findProduct(BAR_CODE)).thenReturn(new Product("beer", 2.3, "0000000001"));

        Product product = dbWorker.findProduct(BAR_CODE);

        Assert.assertEquals(product, PRODUCT);
    }
}

2

Re: Оцінка коду

Подивіться відео, може щось корисноого там для себе візьмете:) https://www.youtube.com/watch?v=CiPhTan … p;index=17
Я поки новачок, але пробую навчатися по цьому курсу, відповідно там є багато чого цікавого!

Подякували: leofun011

3 Востаннє редагувалося truesupport (19.07.2016 14:09:01)

Re: Оцінка коду

Я думаю коли ти робиш printProducts тобі треба зробити порожнім твій ліст  private List<Product> list = new ArrayList<Product>();

також я би робив один метод Print замість printProductsAndTotalPrice,  printProducts, print, printError ну але воно не мішає. Зараз йде тенденція все інжектити і мокати я бачу ви не сильно розумієте як це треба робити. Нажаль я не пишу на джава але ідея всюди одинакова. У вас є одне місце де ви конфігуруєте свої сервіси які потрібно а потім їх вставляєте зазвичай через constructor inejction куди потрібно.  Тобто треба вам якийсь сервіс ви додаєте параметр в конструктор і ваш DI контейнер дасть вам його.

private InputDevice codesScanner = new CodesScanner();
    private DBWorker dbWorker = null;
    private OutputDevice lcdDisplay = new LCDDisplay();
    private OutputDevice printer = new Printer();

оце вері бед, так робити не треба

Має бути щось подібне до такого код на C# але ідея я думаю буде зрозуміла

public interface IScanner
    {
        string ScanBarCode();
    }

    public interface IProductRepository
    {
        Product GetProductByBarCode(string barCode);
    }

    public class Product
    {
        public int ID { get; set; }
        public string ProductName { get; set; }
        public decimal Price { get; set; }
    }

    public interface IDisplayDevice
    {
        DisplayData(string data);
    }

    public class SailPoint
    {
        IScanner _scanner;
        IProductRepository _productRepository;
        IDisplayDevice _lcd;
        IDisplayDevice _printer;
        IList<Product> _products;

        public SailPoint(IScanner scanner, IProductRepository productRepository, IDisplayDevice lcd, IDisplayDevice printer)
        {
            _scanner = scanner;
            _productRepository = productRepository;
            _lcd = lcd;
            _printer = printer;

            _products = new List<Product>();
        }

        public void ProcessInput()
        {
            var barCode = _scanner.ScanBarCode();
            if (barCode != "exit")
            {
                if (barCode == "")
                {
                    _lcd.DisplayData("invalid bar code");
                    return;
                }

                var product = _productRepository.GetProductByBarCode(barCode);
                if (product != null)
                {
                    _lcd.DisplayData(product.ProductName);
                    _products.Add(product);
                }
                else
                {
                    _lcd.DisplayData("product not found");
                }
            }
            else
            {
                FinalizeCheck();
            }
        }

        private void FinalizeCheck()
        {
            decimal totalPrice = 0;

            foreach (var product in _products)
            {
                _printer.DisplayData(product.ProductName + ": " + product.Price);
                totalPrice += product.Price;
            }

            _lcd.DisplayData(totalPrice.ToString());
            _products.Clear();
        }
    }

Це дозволить вам в тестах замокати IProductRepository productRepository наприклад

Але ви повинні розуміти те як я написав це тільки ідея і не факт що там все правильно з точки зору ооп, тому завжди робіть як вам здається правильно.


((Printer)printer).printProductsAndTotalPrice(list, summ);

оце погано, або створюйте принтер, або розширюйте інтерфейс.

Подякували: leofun011