Programowanie obiektowe, wykorzystanie kompozycji, podział na klasy

0

Mam do zrobienia apke, która będzie pobierać z poliku dane nt firm i faktur do opłacenia. Następnie ma być możliwość opłacenia wybranej fv. Opłaceniem fv ma być zmiana pola "status" z 'false' na 'true';
Proszę o rzucenie okiem na moją pracę. Proszę nie wchodzić w szczegóły implementacji poszczególnych metod. Na razie chodzi mi o sprwadzenie, przede wszystkim podziału na klasy, metody, relacje między klasami, wykorzystanie kompozycji itp. Oczywiście jak już to dopracuje to chętnie dowiem się czegoś nt pozostałych elementów.

public class Factur {

    private int facturNumber;
    BigDecimal amountToBePaid;
    private String title;
    private Boolean status;

    public Factur(int facturNumber, BigDecimal amountToBePaid, String title, Boolean status) {
        this.facturNumber = facturNumber;
        this.amountToBePaid = amountToBePaid;
        this.title = title;
        this.status = status;
    }

    public int getFacturNumber() {
        return facturNumber;
    }

    public Boolean getStatus() {
        return status;
    }

    public void setStatus(Boolean status) {
        this.status = status;
    }

    @Override
    public String toString() {
        return "Factur{" +
                "facturNumber=" + facturNumber +
                ", price=" + amountToBePaid +
                ", title='" + title + '\'' +
                ", status='" + status + '\'' +
                '}';
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        Factur factur = (Factur) o;
        return facturNumber == factur.facturNumber &&
                Objects.equals(amountToBePaid, factur.amountToBePaid) &&
                Objects.equals(title, factur.title) &&
                Objects.equals(status, factur.status);
    }

    @Override
    public int hashCode() {
        return Objects.hash(facturNumber, amountToBePaid, title, status);
    }
}

public class Company {

    private String companyName;
    private List<Factur> factursForPay;

    public Company(String companyName, List<Factur> factursForPay) {
        this.companyName = companyName;
        this.factursForPay = factursForPay;
    }

    public String getCompanyName() {
        return companyName;
    }

    public List<Factur> getFactursForPay() {
        return factursForPay;
    }

    @Override
    public String toString() {
        return "Company{" +
                "companyName='" + companyName + '\n' +
                " factursForPay=" + factursForPay + '\n';
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        Company company = (Company) o;
        return Objects.equals(companyName, company.companyName);
    }

    @Override
    public int hashCode() {
        return Objects.hash(companyName);
    }
}


public class Payments {

    private static List<Company> companyList = getCompanyFromDatabase("faktury");

    public Payments() {
    }

   private static List<Company> getCompanyFromDatabase(String filename) {

        List<Company> companyList = new ArrayList<>();
        List<String> newCompany = new ArrayList<>();
        Boolean elExist = false;

        try (
                FileReader fR = new FileReader(filename);
                Scanner sc = new Scanner(fR);) {

            while (sc.hasNextLine()) {
                newCompany = Arrays.asList(sc.nextLine().split(";"));
                elExist = false;

                for (Company elOfCompanyList : companyList) {
                    if (elOfCompanyList.getCompanyName().equals(newCompany.get(0)))
                        elExist = true;
                }

                if (!elExist) {
                    companyList.add(new Company(newCompany.get(0), getFacturList(filename, newCompany.get(0))));
                }
            }

        } catch (Exception e) {
            throw new IllegalArgumentException("FileReader err");
        }
        //System.out.println("From Factursinformationclass getcompany method: " + companyList);
        return List.copyOf(companyList);
    }

  private static List<Factur> getFacturList(String fileName, String companyName) {

        List<Factur> facturList = new ArrayList<>();
        String factur;

        try (
                FileReader fRr = new FileReader(fileName);
                Scanner scc = new Scanner(fRr);) {

            while (scc.hasNextLine()) {
                factur = scc.nextLine();
                if (factur.matches(".{0,}" + companyName + ".{0,}")) {
                    facturList.add(Validation.createFactur(factur));
                }
            }

        } catch (Exception e) {
            throw new IllegalArgumentException("FileReader err");
        }

        return facturList;
    }




    public static List<Company> getCompanyList() {
        return companyList;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        Payments payments = (Payments) o;
        return Objects.equals(companyList, payments.companyList);
    }

    @Override
    public int hashCode() {
        return Objects.hash(companyList);
    }

    @Override
    public String toString() {
        return "Payments{" +
                "companyList=" + companyList +
                '}';
    }
}
public class Validation {

    static Factur createFactur(String facturInformation){

        List<String> fvInfo = Arrays.asList(facturInformation.split(";"));

        if(!fvInfo.get(1).matches("[0-9]{5}")){
            throw new IllegalArgumentException("Factur nr is wrong");
        }

        return new Factur(Integer.valueOf(fvInfo.get(1)),
                 new BigDecimal(fvInfo.get(2)),
                 fvInfo.get(3),
                 Boolean.valueOf(fvInfo.get(4)));
    }


}
class FactursManager {

    private static Map<Factur, Boolean> facturStatus = new HashMap<>();

    private static void refreshFacturInfo(List<Company> companyList) {

        //-------------Tutaj dodałbym zapis do pliku nowych info o statusie
        //poprzez czytanie starego pliku, aktualizacja statusu i zapis do listy.
        //Następnie lista do pliku;

        for (Company elOfCompanyList : companyList) {
            for (Factur elOfFacturList : elOfCompanyList.getFactursForPay()) {
                facturStatus.put(elOfFacturList, elOfFacturList.getStatus());
            }
        }
        System.out.println("Recently payment's status");
        for (Map.Entry<Factur, Boolean> elOfMap : facturStatus.entrySet()) {
            System.out.println(elOfMap);
        }
    }

    static void makePeyment() {

        List<Company> companyList = Payments.getCompanyList();
        System.out.println("Company List: " + companyList);
        int choose = Integer.MIN_VALUE;

        do {
            choose = UserDataService.getInt("1 --> Pay || anyButton --> Exit");
            if(choose == 1) {
            int fvNr = UserDataService.getInt("Factur number for pay: ");
                for (Company elOfCompanyList : companyList) {
                    for (Factur elOfFvList : elOfCompanyList.getFactursForPay()) {
                        if (elOfFvList.getFacturNumber() == fvNr) {
                            transferMoney(fvNr, companyList);
                        }
                    }
                }
                refreshFacturInfo(companyList);
            }
        } while (!(choose == 2));
    }

    private static void transferMoney(int fvNumber, List<Company> companyList) {

        for (Company elOfCompanyList : companyList) {
            for (Factur elOfFacturList : elOfCompanyList.getFactursForPay()) {
                if (elOfFacturList.getFacturNumber() == fvNumber) {
                    elOfFacturList.setStatus(true);
                    System.out.println("transfer complet");
                }
            }
        }
    }
}

public class Main {
    public static void main(String[] args) {
        FactursManager.makePeyment();
    }

}
2

Według mnie to:

  1. Jak masz encje/obiekty domenowe to klasy do pobierania tych obiektów powinny znajdować się w dedykowanych klasach DAO/Repository.
    U Ciebie są to jakieś statyczne metody np. w Payments.
  2. Inicjalizacja List<Company> companyList = getCompanyFromDatabase("faktury"); ja bym inicjalizował przez konstruktor.
  3. Hashe, equalsy i settery to lombokiem bym cisnął.
  4. Jak masz klasę Company to jak nadajesz takiej firmie tożsamość? Przecież dwie firmy teoretycznie mogą nazywać się tak samo / podobnie. Do każdego obiektu domenowego dorzuciłbym ID.
  5. Jakbym pisał walidator to używałbym dla formalności Predicate<T>
  6. FactursManager nie powinien IMHO zawierać metod statycznych tylko być zrobiony na modłe interfejsu serwisowego.
    Używanie FacturManager powinno odbywać się na instancji obiektu.
    7 Wydaje mi się że to Firma ma płatności a nie płatności mają firmę. Według mnie powinno być List<Payments> w Company.
2

Dodaj po trzech odwrotnych apostrofach słowo java to powinno pokolorować składnię
Dlaczego w Factur używasz Boolean dla status zamiast boolean ?
Nie bardzo rozumiem sens zmiennej Map<Factur, Boolean> facturStatus.

0
MrMadMatt napisał(a):

Według mnie to:

  1. Jak masz encje/obiekty domenowe to klasy do pobierania tych obiektów powinny znajdować się w dedykowanych klasach DAO/Repository.
    U Ciebie są to jakieś statyczne metody np. w Payments.
  2. Inicjalizacja List<Company> companyList = getCompanyFromDatabase("faktury"); ja bym inicjalizował przez konstruktor.
  3. Hashe, equalsy i settery to lombokiem bym cisnął.
  4. Jak masz klasę Company to jak nadajesz takiej firmie tożsamość? Przecież dwie firmy teoretycznie mogą nazywać się tak samo / podobnie. Do każdego obiektu domenowego dorzuciłbym ID.
  5. Jakbym pisał walidator to używałbym dla formalności Predicate<T>
  6. FactursManager nie powinien IMHO zawierać metod statycznych tylko być zrobiony na modłe interfejsu serwisowego.
    Używanie FacturManager powinno odbywać się na instancji obiektu.
    7 Wydaje mi się że to Firma ma płatności a nie płatności mają firmę. Według mnie powinno być List<Payments> w Company.
  1. Czytanie z pliku przeniosłem do klasy "FileService"
  2. Poprawione
  3. To na razie zostawię sobie tak jak jest.
  4. Dokładnie jakieś ID lub NIP firmy powinien być dodany ale to następnym razem ;)
  5. Czy coś w tym stylu może być ??
  6. Spójrz do kodu
  7. Na razie pominąłem
class FactursManager {
    
    public FactursManager(){}

    private void refreshFacturInfo(List<Company> companyList) {

        //-------------Tutaj dodałbym zapis do pliku nowych info o statusie
        //poprzez czytanie starego pliku, aktualizacja statusu i zapis do listy.
        //Następnie lista do pliku;

        if (companyList == null) {
            throw new IllegalArgumentException("companyList is null");
        }

        System.out.println("----------------");
        for (Company elOfCompanyList : companyList) {
            System.out.println(elOfCompanyList);
        }
    }
    
    void makePeyment() {

        Payments payments = new Payments();
        List<Company> companyList = payments.getCompanyList();
        System.out.println("Company List: " + companyList);
        int choose = Integer.MIN_VALUE;

        do {
            choose = UserDataService.getInt("1 --> Pay || anyButton --> Exit");
            if(choose == 1) {
            int fvNr = UserDataService.getInt("Factur number for pay: ");
                for (Company elOfCompanyList : companyList) {
                    for (Factur elOfFvList : elOfCompanyList.getFactursForPay()) {
                        if (elOfFvList.getFacturNumber() == fvNr) {
                            transferMoney(fvNr, companyList);
                        }
                    }
                }
                refreshFacturInfo(companyList);
            }
        } while (!(choose == 2));
    }

    private void transferMoney(int fvNumber, List<Company> companyList) {

        for (Company elOfCompanyList : companyList) {
            for (Factur elOfFacturList : elOfCompanyList.getFactursForPay()) {
                if (elOfFacturList.getFacturNumber() == fvNumber) {
                    elOfFacturList.setStatus(true);
                    System.out.println("transfer complet");
                }
            }
        }
    }
}

public class FileService {

    public static List<String> readFile(String filename) {

        if (filename == null) {
            throw new IllegalArgumentException("filename is null");
        }
        String txt = null;
        List<String> readedFromFile = new ArrayList<>();

        try (FileReader fileReader = new FileReader(filename); Scanner sc = new Scanner(fileReader)) {
            while (sc.hasNextLine()) {
                readedFromFile.add(sc.nextLine());
            }
            return readedFromFile;
        } catch (Exception e) {
            throw new IllegalStateException("read file exception: " + e.getMessage());
        }
    }
}

public class Payments {

    private List<Company> companyList;

    public Payments() {
        companyList = getCompanyFromDatabase("faktury");
    }

    private List<Company> getCompanyFromDatabase(String filename) {

        List<Company> companyList = new ArrayList<>();
        List<String> newCompany = new ArrayList<>();
        Boolean elExist = false;

        List<String> dataFromFile = FileService.readFile(filename);

        for (String elOfDataFromFile : dataFromFile) {
            newCompany = Arrays.asList(elOfDataFromFile.split(";"));
            elExist = false;
            for (Company elOfCompanyList : companyList) {
                if (elOfCompanyList.getCompanyName().equals(newCompany.get(0)))
                    elExist = true;
            }
            if (!elExist) {
                companyList.add(new Company(newCompany.get(0), getFacturList(filename, newCompany.get(0))));
            }
        }
        return companyList;
    }

    private List<Factur> getFacturList(String fileName, String companyName) {

        List<Factur> facturList = new ArrayList<>();
        List<String> facturInfo = FileService.readFile(fileName);

        for (String elOfFacturInfoList : facturInfo) {
            if (elOfFacturInfoList.matches(".{0,}" + companyName + ".{0,}")) {
                facturList.add(Validation.createFactur(elOfFacturInfoList));
            }
        }
        return facturList;
    }


    public List<Company> getCompanyList() {
        return companyList;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        Payments payments = (Payments) o;
        return Objects.equals(companyList, payments.companyList);
    }

    @Override
    public int hashCode() {
        return Objects.hash(companyList);
    }

    @Override
    public String toString() {
        return "Payments{" +
                "companyList=" + companyList +
                '}';
    }
}
public class Validation {

    static Factur createFactur(String facturInformation){

        List<String> fvInfo = Arrays.asList(facturInformation.split(";"));
        List<String> facturNumber = List.of(fvInfo.get(1));

        Predicate<String> p = (s -> s.matches("[0-9]{5}"));

        if(!(facturNumber.stream().allMatch(p))){
            throw new IllegalArgumentException("Fv nr is wrong");
        }

        return new Factur(Integer.valueOf(facturNumber.get(0)),
                 new BigDecimal(fvInfo.get(2)),
                 fvInfo.get(3),
                 Boolean.valueOf(fvInfo.get(4)));
    }
}

public class Main {
    public static void main(String[] args) {
        //FactursManager.makePeyment();
        FactursManager factursManager = new FactursManager();
        factursManager.makePeyment();
    }

}
1
  1. Z repozytorium/DAO chodziło mi o to abyś metody:
private static List<Company> getCompanyFromDatabase(String filename)
private static List<Factur> getFacturList(String fileName, String companyName) 

Przerzucił do nowej klasy np. CompanyRepository oraz FacturRepository i miał nową klasę w stylu:

class CompanyRepository {
    String fileName;

    List<Company> getCompanyFromDatabase();
    List<Company> getCompanyList();
}

class FacturRepository {
    String fileName;

  List<Factur> getFacturList( String companyName) 
        ...
    }
}

Tak abyś miał dedykowane klasy do pobrania danego rekordu z bazy.

  1. ten Validator to tak na prawdę FacturFactory, ma jak dla mnie mylącą nazwę bo ona nie sprwadza danych tylko je tworzy.

  2. Ten Payments to do czego ma służyć? To rekord z bazy czy co?

  3. FactursManager wygląda dla mnie ok tylko ja bym to nazywał FactursService albo coś w tym guście.

0

P.S.
Czy znacie może miejsca gdzie mógłbym podpatrzeć kody ludzi znających się na rzeczy ?? Tylko wiecie, wehikułu do cofania się w czasie nie ogarne ;)

0
MrMadMatt napisał(a):
  1. Z repozytorium/DAO chodziło mi o to abyś metody:
private static List<Company> getCompanyFromDatabase(String filename)
private static List<Factur> getFacturList(String fileName, String companyName) 

Przerzucił do nowej klasy np. CompanyRepository oraz FacturRepository i miał nową klasę w stylu:

class CompanyRepository {
    String fileName;

    List<Company> getCompanyFromDatabase();
    List<Company> getCompanyList();
}

class FacturRepository {
    String fileName;

  List<Factur> getFacturList( String companyName) 
        ...
    }
}

Tak abyś miał dedykowane klasy do pobrania danego rekordu z bazy.

  1. ten Validator to tak na prawdę FacturFactory, ma jak dla mnie mylącą nazwę bo ona nie sprwadza danych tylko je tworzy.

  2. Ten Payments to do czego ma służyć? To rekord z bazy czy co?

  3. FactursManager wygląda dla mnie ok tylko ja bym to nazywał FactursService albo coś w tym guście.

  1. Sprawdza dzięki
 Predicate<String> p = (s -> s.matches("[0-9]{5}"));

        if(!(facturNumber.stream().allMatch(p))){
            throw new IllegalArgumentException("Fv nr is wrong");
        }

i jeśli nie rzuci wyjątku to utworzy obiekt

  1. Ten Payments to szczerze mówiąc w zadaniu był tak zaproponowany. Może dlatego, żeby lepiej odwzorować rzeczywistość? Żeby oddzielić klasę pojemnik od operacji wykonywanych na obiektach z tej klasy??
1
manifestor napisał(a):
  1. Sprawdza dzięki
 Predicate<String> p = (s -> s.matches("[0-9]{5}"));

        if(!(facturNumber.stream().allMatch(p))){
            throw new IllegalArgumentException("Fv nr is wrong");
        }

Faktycznie przeoczyłem linijkę walidacji aczkolwiek według mnie dalej jest to myląca nazwa, bo patrz:

// Klasa nazywa sie walidator
public class Validation {

    // metoda wystawiana przez klase to create, czyli tworzenia. A czy walidator sluzy do tworzenia czy sprwadzania danych?
    static Factur createFactur(String facturInformation){

        List<String> fvInfo = Arrays.asList(facturInformation.split(";"));
        List<String> facturNumber = List.of(fvInfo.get(1));

        // tu jest predykat na sztywno zespawany z fabryka
        Predicate<String> p = (s -> s.matches("[0-9]{5}"));

        if(!(facturNumber.stream().allMatch(p))){
            throw new IllegalArgumentException("Fv nr is wrong");
        }

        return new Factur(Integer.valueOf(facturNumber.get(0)),
                 new BigDecimal(fvInfo.get(2)),
                 fvInfo.get(3),
                 Boolean.valueOf(fvInfo.get(4)));
    }
}

Jak miałbym się czepiać, oczywiście nie ze względu na złą wolę to ja bym zaproponował coś takiego:

public class FacturFactory {

    // predykat przez konstruktor aby nie uzalezniac fabryki od predykatu, w takiej opcji mozna testowac/podmieniac sam predykat
    Predicate<String> p;

    Factur createFactur(String facturInformation){

        List<String> fvInfo = Arrays.asList(facturInformation.split(";"));
        List<String> facturNumber = List.of(fvInfo.get(1));

        // nie jestem pewny czy tego wyjatku nie powinno sie zadeklarowac przy sygnaturze metody 
        // tak aby dać info użytkownikowi metody że coś może pójść nietak.
        if(!(facturNumber.stream().allMatch(p))){
            throw new IllegalArgumentException("Fv nr is wrong");
        }

        return new Factur(Integer.valueOf(facturNumber.get(0)),
                 new BigDecimal(fvInfo.get(2)),
                 fvInfo.get(3),
                 Boolean.valueOf(fvInfo.get(4)));
    }
}


class FactoryPredicate implements Predicate<String> {

    boolean test(String s) {
        return (s -> s.matches("[0-9]{5}"));
    }
}
// przy tworzeniu
Predicate<String> factoryPredicate = new FactoryPredicate();
FacturFactory factory = new FacturFactory(factoryPredicate)

W moim rozwiązaniu ograniczasz złożoność między fabryką a predykatami, ktoś może się nie zgodzić że pcham predykaty do klas anie do lambd aczkolwiek jak zawsze to zależy, jeden rabin powie tak, inny powie nie. ;)

Co do pytania o patrzenie kodu, najlepiej jakiś Github z projektem który służył np. w kursie udemy za code example.

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