- Pliki konfiguracyjne zbędne
- Pliki impl zbędne
- Brak infomacji jak odpalić
- Nazwa nie sugeruje to co ma mówić:
ControlApp controlApp = new ControlApp();
controlApp.hotelControl();
}
- Jesteś pewny, że chcesz tylko po przez konstruktor wstrzykiwać zależności?
public ControlApp() {
hotel = new Hotel();
dataReader = new DataReader();
}
-
Field nie wkszuje na to co robi:
private Hotel hotel
-
Wcięcie
}
public void hotelControl() {
Option option = null;
-
Dlaczego nie jako pole? I tak go nullujesz
Option option = null;
-
A potem sprawdzasz co zawiera?
while (option != Option.EXIT)
-
Można by go przerzucić do throw metody, ładniej by to wyglądało
try {
printOptions();
option = Option.createFromInt(dataReader.readInt());
switch (option) {
case DISPLAY_ALL_ROOMS:
printAllRooms();
break;
case DISPLAY_FREE_ROOMS:
printFreeRooms();
break;
case DISPLAY_ROOM:
printRoom();
break;
case RENT_ROOM:
rentRoom();
break;
case RELEASE_ROOM:
releaseRoom();
break;
case EXIT:
break;
default:
System.out.println("Brak opcji o takim numerze");
}
}catch (InputMismatchException e){
System.out.println("Wrowadzona liczba ma niepoprawny format");
}catch ( NoSuchElementException e) {
System.out.println("Brak opcji o takim numerze");
}
}
-
Użyłbym strategii.
switch (option) {
case DISPLAY_ALL_ROOMS:
printAllRooms();
break;
case DISPLAY_FREE_ROOMS:
printFreeRooms();
break;
case DISPLAY_ROOM:
printRoom();
break;
case RENT_ROOM:
rentRoom();
break;
case RELEASE_ROOM:
releaseRoom();
break;
case EXIT:
break;
default:
System.out.println("Brak opcji o takim numerze");
}
-
Przerzuciłym to do osobnej klasy np: DataReaderStrategy. Metoda nie mówi tego co robi
-
Przerzuciłbym to do osobnej klasy np: ConsolePrinter i na podstawi akcji tworzyłbym event. (Poczytaj o strategii)
private void printOptions() {
System.out.println("\nWybierz opcje");
for (Option o : Option.values()) {
System.out.println(o);
}
}
private void printAllRooms() {
for (int i = 1; i <= Hotel.getMaxRooms(); i++) {
System.out.println(hotel.getRoom(i));
}
}
private void printFreeRooms() {
long count = hotel.getRooms().values().stream().filter(x -> x.getLastName()==null)
.sorted(new Hotel.sortByNr()).peek(System.out::println).count();
}
private void printRoom() {
System.out.println("Podaj numer pokoju 1...25");
Room p = hotel.getRoom(dataReader.readInt());
if (p == null) {
System.out.println("Nie ma takiego pokoju");
} else
System.out.println(p);
}
- Można jako osobną kolekcje metod: + plus metoda nie mówi tego co robi
long count = hotel.getRooms().values().stream().filter(x -> x.getLastName()==null)
.sorted(new Hotel.sortByNr()).peek(System.out::println).count();
- Za duża odpowiedzialność klasy, mógłbyś pociągnić pod klasę typu serwis: RentService
private void rentRoom() {
Room p = dataReader.readPerosn();
if(p.getRoomNumber()>25) {
System.out.println("Nie ma takiego pokoju");
}else
hotel.rentRoom(p);
}
private void releaseRoom(){
System.out.println("Podaj dane wynajmującego");
Room p = dataReader.readPerosn();
String fullName = p.getFullName();
String roomNr= Integer.toString(p.getRoomNumber());
if(hotel.getRooms().get(fullName).equals(hotel.getRooms().get(roomNr))) {
hotel.releaseTheRoom(p);
}else
System.out.println("Nie ma takiej osoby");
}
-
Na pierwszy rzut oka, ten if nic nie mówi. Powinno być osobną metodą. Poczytaj o kompozyt.
if(p.getRoomNumber()>25) {
if(hotel.getRooms().get(fullName).equals(hotel.getRooms().get(roomNr)))
- Dlaczego enum jest wraz hotelControl ?
public enum Option {
EXIT(0, "Wyjście"),
DISPLAY_ALL_ROOMS(1, "Wyświetl wszystkie pokoje"),
DISPLAY_FREE_ROOMS(2, "Wyświetl wolne pokoje"),
DISPLAY_ROOM(3, "Wyświetl informacje o pokoju"),
RENT_ROOM(4, "Wynajmij pokój"),
RELEASE_ROOM(5, "Zwolnij pokój");
private int value;
private String description;
Option(int value, String description) {
this.value = value;
this.description = description;
}
-
??? . Poczytaj o mapowaniu.
blic static Option createFromInt(int i) throws NoSuchElementException, ArrayIndexOutOfBoundsException {
Option result = null;
try {
result = Option.values()[i];
} catch (NoSuchElementException | ArrayIndexOutOfBoundsException e) {
throw new NoSuchElementException("Brak opcji o wskazanym numerze");
}
return result;
}
-
To samo co wcześniej, za duża odpowiedzialność:
public class Hotel
-
Co zostało podektywowane za tym rozwiązaniem?
room = new TreeMap<>();
-
Możesz również to za pomocą filter.
public static class sortByNr implements Comparator<Room> {
@Override
public int compare(Room p1, Room p2){
if(p1==null && p2==null) return 0;
if(p1==null) return 1;
if(p2==null) return -1;
Integer i1 = p1.getRoomNumber();
Integer i2 = p2.getRoomNumber();
return i1.compareTo(i2);
}
}
Poczytaj o standardach o strukturze javowych projektów
