Obliczenie rabatu dla każdej pozycji zamówienia

0

Witam, zaczynam swoją przygodę z programowaniem obiektowym i utknąłem w pewnym momencie swojego programu. Mam dwie klasy Pozycja i Zamówienie, w klasie Pozycja posiadam pola: cena, ilość, nazwa towaru. W Zamówieniu robię wszystkie inne rzeczy typu: obliczenie wartości produktu, całego zamówienia oraz rabatu w zależności od ilości sztuk. W moim przypadku metoda calculateDiscount liczy wartość zamówienia dla wszystkich pozycji w zależności od ilości sztuk. Pomyślałem, że chciałbym mieć możliwość udzielenia rabatu na konkretną pozycję. Tutaj mam problem, bo totalnie nie wiem jak zaimplementować taką metodę w klasie zamówienie. Proszę o podpowiedź.

package programming.com.pl;

public class Position  {

    private String name;
    private double price = 0;
    private int quantity = 0;


    public Position(String name, double price, int quantity){
        this.name = name;
        this.price = price;
        this.quantity = quantity;
    }

    public double getPrice() {
        return price;
    }

    public int getQuantity() {
        return quantity;
    }

    public String toString(){
        String str = String.format("%4s,%4s zł,%2s szt.", name,price,quantity);
        return  str;
    }
}
package programming.com.pl;

import java.util.ArrayList;

public class Order {
    final private ArrayList<Position> positions = new ArrayList<>();

    private double calculateProduct() {
        double sum = 0;
        for (Position position : positions) {
            sum = position.getPrice();
        }
        return sum;
    }

    double sumOrder() {
        double sum = 0;
        for (Position x : positions) {
            sum += calculateProduct();
        }
        return sum;
    }


    void addPosition(Position p) {
        positions.add(p);
    }

    void deletePosition(int index) {
        positions.remove(index);
    }




    double calculateDiscount() {
        double salePrice = 0;
        for (Position position : positions) {
            if (position.getQuantity() >= 5 && position.getQuantity() <= 10) {
                salePrice = calculateProduct() - (calculateProduct() * 0.05);
            } else if (position.getQuantity() <= 10 && position.getQuantity() <= 20) {
                salePrice = calculateProduct() - (calculateProduct() * 0.1);
            } else {
                salePrice = calculateProduct() - (calculateProduct() * 0.15);
            }

        }

        return salePrice;
    }
    public String toString() {
        StringBuilder sb = new StringBuilder("Order is: \n");

        for (Position p : positions) {
            sb.append(p).append("\n");
        }
        sb.append("Order sum is: ").append(sumOrder()).append("\n");
        sb.append("Discount is: ").append(calculateDiscount());
        return sb.toString();
    }
}

3
crav21 napisał(a):

Witam, zaczynam swoją przygodę z programowaniem obiektowym i utknąłem w pewnym momencie swojego programu.

Jeśli na prawdę chciałbyś żeby to był obiektowy kod, to obliczanie ceny konkretnego produktu powinno być w Position, bo u Ciebie ta klasa jest tylko pojemnikiem na dane, bardziej strukturą niż obiektem.

Wielu ludzi wpada w pułapkę ponieważ myślą że "po prostu używanie klas" to programowanie obiektowe, bzdura.

  1. Nie operuj na pieniądzach na typie double, używaj int albo innego typu całkowitego. Tutaj masz to bardzo dobrze wyjaśnione Why not use Double or Float to represent currency?.
  2. Funkcja calculateDiscount() jest przekombinowana na 10tą stronę, a mogłaby być bardzo prosta. Zobacz jak funkcja getDiscountedPrice() jest prosta, mimo że robi dokładnie to samo co Twoja calculateDiscount().

To by był na prawdę obiektowy kod

 class Position  {
    private final String name;
    private final double price;
    private final int quantity;

    public Position(String name, double price, int quantity){
        this.name = name;
        this.price = price;
        this.quantity = quantity;
    }

    public double getDiscountedPrice() {
        if (quantity < 5) {
            return price;
        }
        if (quantity <= 10) {
            return price * 0.95;
        } 
        if (quantity <= 20) {
            return price * 0.9;
        }
        return price * 0.85;
    }
}

class Order {
    private final List<Position> positions = new ArrayList<>();

    double getPrice() {
        double sum = 0;
        for (Position position : positions) {
            sum += position.getDiscountedPrice();
        }
        return sum;
    }

    void add(Position position) {
        positions.add(position);
    }

    void deletePosition(int index) {
        positions.remove(index);
    }
}

Chociaż bardziej obiektowo byłoby gdyby Order przyjmował positions przez konstruktor i był immutable.

1

Mozesz skomplikowac sobie zadanie, aby pocwiczyc OOP i wzorzec strategii. Zalozmy, ze sa 2 rodzaje rabatow:

  1. Ilosciowy: kup N produktow, aby dostac znizke M%
  2. Zestawy: kup produkt X i Y, aby dostac znizke Z%

Dodatkowe utrudnienie: znizki nie lacza sie

0

Szczerze mówiąc to mnie zamurowało. Generalnie zastanawiałem się jak to ugryźć. Spytałem kogoś kto programuje bardzo długo i dostałem taką podpowiedź:
"Uwazam ze pozycja to konkretny przedmiot i dopiero w zamowienie decyduje o ilosci". Sam nie wiedziałam czy ilość to cecha zamówienia czy pozycji. Częsty problem jak zbudować dana klasę..

1

Kolega ma racje. Przeciez jak masz produkt w katalogu (np. na listingu ofert na Allegro), to on nie ma jeszcze kontekstu zamowienia, wiec nie ma mowy o ilosci (wlasciwie liczbie).

1
crav21 napisał(a):

Szczerze mówiąc to mnie zamurowało. Generalnie zastanawiałem się jak to ugryźć. Spytałem kogoś kto programuje bardzo długo i dostałem taką podpowiedź:
"Uwazam ze pozycja to konkretny przedmiot i dopiero w zamowienie decyduje o ilosci". Sam nie wiedziałam czy ilość to cecha zamówienia czy pozycji. Częsty problem jak zbudować dana klasę..

Tak na prawdę oba pojęcia są okej. Sam nie wiedziałam czy ilość to cecha zamówienia czy pozycji to nie ma znaczenia. Nie próbuj odnosić "prawdziwego świata" to impementacji, to droga do nikąd.

Najlepsza rada dla Ciebie to zrób dwie implementacje (np na dwóch branchach), jedna gdzie ilość jest w Position, jedna gdzie ilość jest w Order i zobacz która Ci się bardziej podoba.

Nie dziel kodu na klasy "tak jak są podzielone w prawdziwym świecie", tylko tak żeby to podzielenie miało programistyczny sens, spełaniło SRP, open/close, były łatwo testowalne, były loosely-coupled, etc. Jakiekolwiek "modelowanie rzeczywistości" to mit.

0

dodatkowo jeszcze taki przykład mi podał: "Wyobraz sobie ze robisz tak ze masz w klasie pozycja nazwe, cene i ilosc. Nastepnie klient przychodzi i mowi ze chcialby dodac jeszcze dodac mozliwosc doliczania rabatu, ale tylko na niektore produkty, teoretycznie mozesz znowu dodac rabat w klasie pozycja, najwyzej czesto bedzie zerowy. Potem klient mowi ze chcialby zeby rabat naliczal sie w zaleznosci od tego jakie produkty znajdują sie w zamowieniu i jesli zamowienie posiada minimum 3 produkty z kategorii premium to chcemy dodac 10% rabatu. I teraz co? Przy kazdym dodawaniu i odejmowaniu do koszyka bedzie trzeba iterowac sie po wszystkich pozycjach u ustawiac albo zerowac rabaty, strach pomyslec co sie stanie jak niektore produkty beda mialy miec obnizke czasową i promocje mają sie nie łączyć"

1
crav21 napisał(a):

dodatkowo jeszcze taki przykład mi podał: "Wyobraz sobie ze robisz tak ze masz w klasie pozycja nazwe, cene i ilosc. Nastepnie klient przychodzi i mowi ze chcialby dodac jeszcze dodac mozliwosc doliczania rabatu, ale tylko na niektore produkty, teoretycznie mozesz znowu dodac rabat w klasie pozycja, najwyzej czesto bedzie zerowy. Potem klient mowi ze chcialby zeby rabat naliczal sie w zaleznosci od tego jakie produkty znajdują sie w zamowieniu i jesli zamowienie posiada minimum 3 produkty z kategorii premium to chcemy dodac 10% rabatu. I teraz co? Przy kazdym dodawaniu i odejmowaniu do koszyka bedzie trzeba iterowac sie po wszystkich pozycjach u ustawiac albo zerowac rabaty, strach pomyslec co sie stanie jak niektore produkty beda mialy miec obnizke czasową i promocje mają sie nie łączyć"

Brzmi jak nierozsądna rada, i to z kilku powodów:

  1. Po pierwsze, to jak wygląda implementacja teraz, nie jest wypisane w kamieniu. Jeśli wymagania co do programu się zmienią, rozwiązanie należy zrefaktorować tak by było odpowiednie.
  2. Po drugie, nie jesteś jasnowidzem, więc nie możesz przewidzieć w całości jaki przyjdzie requirement. Stosuj YAGNI.
  3. Po trzecie, nawet gdybyś chciał przygotować odpowiednią abstrakcję, żeby zabezpieczyć się na dodawanie kolejnych requirementów, to nie znasz osi rotacji tych wymagań, więc jesteś w stanie przewidzieć jaka abstrakcja będzie odpowiednia.

Potem klient mowi ze chcialby zeby rabat naliczal sie w zaleznosci od tego jakie produkty znajdują sie w zamowieniu i jesli zamowienie posiada minimum 3 produkty z kategorii premium to chcemy dodac 10% rabatu. I teraz co? Przy kazdym dodawaniu i odejmowaniu do koszyka bedzie trzeba iterowac sie po wszystkich pozycjach u ustawiac albo zerowac rabaty,

No wtedy należałoby najprawdopodobniej obliczanie rabatu wsadzić do Order, nie do Position, proste. Ale wtedy kiedy będzie na to requirement, nie koniecznie teraz.

0

Super, dziękuje otworzyło mi to trochę perspektywę. Jeśli zdecydowałbym się na obliczenie rabatu dla każdej pozycji w Klasie Order to jest to wykonalne? Czy pozostaje mi zgodnie z Twoim przykładem obliczenie rabatu w Pozycji.

0
crav21 napisał(a):

Super, dziękuje otworzyło mi to trochę perspektywę. Jeśli zdecydowałbym się na obliczenie rabatu dla każdej pozycji w Klasie Order to jest to wykonalne? Czy pozostaje mi zgodnie z Twoim przykładem obliczenie rabatu w Pozycji.

No oczywiście że jest wykonalne.

Dobierz odpowiednią implementacje pod zadanie, to czy jest w tej klasie czy w drugiej, tak długo jak te klasy spełaniją dobre praktyki, to jest wszystko ok.

0

jedyna opcja jaka przychodzi na myśl to w metodzie, która liczy rabat umieścić indeks i odwołać się przez listę za pomocą get. Nie wiem czy to zdrowe podejście

0
crav21 napisał(a):

jedyna opcja jaka przychodzi na myśl to w metodzie, która liczy rabat umieścić indeks i odwołać się przez listę za pomocą get. Nie wiem czy to zdrowe podejście

Raczej nie, bo uzależniasz implementację od kolejności elementów i tego że są trzymane w sekwencyjnej kolekcji. Przekaz referencje do Position po prostu.

@crav21: Napisz dokładnie co ten program miałby robić to pomożemy.

0

liczyć rabat dla zamówienia, dla każdej pozycji w zależności od ilości sztuk również liczyć rabat. Tak dodatkowo, jeśli dodawana pozycja się powtórzy ma zwiększać ilość sztuk.

0
crav21 napisał(a):

liczyć rabat dla zamówienia, dla każdej pozycji w zależności od ilości sztuk również liczyć rabat. Tak dodatkowo, jeśli dodawana pozycja się powtórzy ma zwiększać ilość sztuk.

No ale przecież to obliczanie rabatu jest załatwione przez tą wersję: (post) https://4programmers.net/Forum/Java/358620-obliczenie_rabatu_dla_kazdej_pozycji_zamowienia?p=1824612#id1824612

Chyba że chodzi Ci o coś takiego?

order = new Order();
order(new Position("Masło", 2, 10));

vs

order = new Order();
order(new Position("Masło", 1, 10));
order(new Position("Masło", 1, 10));
1

No ale przecież to obliczanie rabatu jest załatwione przez tą wersję: (post) Obliczenie rabatu dla każdej pozycji zamówienia

Nie jestem przekonany, że Position powinno znać koncept Discounta - tutaj zależnosc jest raczej w drugą stronę. Niemniej jednak, jeśli to zadanie „szkolne”, to nie ma co komplikować.

0
Charles_Ray napisał(a):

No ale przecież to obliczanie rabatu jest załatwione przez tą wersję: (post) Obliczenie rabatu dla każdej pozycji zamówienia

Nie jestem przekonany, że Position powinno znać koncept Discounta - tutaj zależnosc jest raczej w drugą stronę.

Mając taki requirement jak obliczanie rabatu (bo na razie tylko o takim wiemy) to zależności w obie strony są poprawne. Preferowanie jedna nad drugą, mając tylko ten jeden argument jest arbitralne i nieobiektywne w gruncie rzeczy.

0

Dziękuję za ten kod, ale chciałem właśnie umieścić to w klasie zamowienie tak dla treningu. Tylko muszę rozpracować kod z przekazaniem obiektu do konstruktora.

0
crav21 napisał(a):

Dziękuję za ten kod, ale chciałem właśnie umieścić to w klasie zamowienie tak dla treningu. Tylko muszę rozpracować kod z przekazaniem obiektu do konstruktora.

No spoko, tylko napisałeś że ma być obiektowo w pierwszym poście, więc jak tak zrobisz; to Twoja klasa Position będzie trzymała 3 pola bez żadnej logiki i enkapsulacji (bo zrobienie gettera getQuantity() na pole quantity to nie enkapsulacja).

To równie dobrze mógłbyś w ogóle tej klasy Position nie mieć.

No, to przerób ten kod; a jak będziesz miał jakieś pytania to pisz.

0

Powiedzmy, że zadanie jest treningowe, nie chce wprowadzać chaosu. Czy przekazanie przez Ciebie pozycji do konstruktora miało znamię obliczania rabatu w klasie Order?

0
crav21 napisał(a):

Powiedzmy, że zadanie jest treningowe, nie chce wprowadzać chaosu. Czy przekazanie przez Ciebie pozycji do konstruktora miało znamię obliczania rabatu w klasie Order?

Nie, to było po to żeby Order było immutable, czyli bardziej obiektowe.

0

Pomyślałem, że chciałbym mieć możliwość udzielenia rabatu na konkretną pozycję.

Tak sobie analizuje problem i trochę mi tu pasuje wzorzec dekorator.

Czyli mamy klasę, która jest np. rabatem na konkretne grupy produktów i nią opakowujemy zamówienie robiąc np.:

Order order = new Order()
//tworzymy zamówienie
OrderWithDoscount orderWithDiscount = new OrderWithDiscount(order) //nakładamy rabat

W ten sposób można niejako nakładać na siebie różne rabaty.

Oczywiście te rzeczy byśmy mieli w jakimś serwisie DiscountService.

2

TL;DR Rabat nalicza sie od zamowienia a nie od pozycji.
Od pojedynczych pozycji mozesz miec obnizke ceny.
Jaka to roznica? Jak masz ceny z groszami i policzysz od pozycji rabat to wyjdzie inna wartosc (po zaokragleniu pozycji) niz gdy wyliczasz rabat dla calego zamowienia
A modeli rabatów jest 1500.
Przykladowy: 13% rabatu od zamowienia jesli wartosc zamowienia przekracza 100 zl

0

Chciałem jeszcze dopytać kwestię immutable, co daje nam przekazanie przez konstruktor obiekt typu Position?

1
crav21 napisał(a):

Chciałem jeszcze dopytać kwestię immutable, co daje nam przekazanie przez konstruktor obiekt typu Position?

No jeśli obiekt ma być immutable, to nie może mieć takich funkcji jak addPosition() czy removePosition(), więc trzeba je usunąć. Jak więc w inny sposób wsadzić Position do Order, jeśli nie przez konstruktor.

0

Dziękuje :)

1 użytkowników online, w tym zalogowanych: 0, gości: 1