2012-11-07 3 views
0

Я проектирую простую систему проката автомобилей и задавался вопросом, пользуюсь ли я хорошей практикой для дизайна. По сути, у меня есть мастер-салоник, в котором хранятся все арендные автомобили (новые объекты). Внутри каждого автомобильного объекта есть арраист за доступность этого автомобиля за этот месяц. Доступность включает 31 значение (соответствующее каждому дню в этом месяце), либо 0, либо 1 (доступно). Первоначально они установлены на 1, пока автомобиль не будет забронирован на этот день. Есть ли другой способ включения доступности?Java - Это хорошая практика?

ArrayList<Car> showroom = new ArrayList<Car>(); 

ArrayList<Integer> Available1 = new ArrayList<Integer>(); 
ArrayList<Integer> Available2 = new ArrayList<Integer>(); 

setAllDatesAvailable(Available1); 
setAllDatesAvailable(Available2); 

Car number1 = new Car(objectitems, ... , Available1); 
showroom.add(number1); 
Car number2 = new Car(objectitems, ... , Available2); 
showroom.add(number2); 

// Бронирование процесс

setAllDatesAvailable(ArrayList Array) { 
    for (int i = 0; i < 31; i++) { 
     Array.add(1); 
} 

NB:

  • я не волнуюсь по поводу меток времени, предположим, что автомобиль получает желтую карточку за весь день
  • предполагают, что система предназначена только для греха gle месяц 31 дней
+4

Удалите дверь [codereview.se] с этим вопросом. –

+1

начните с изменения деклараций на список, интерфейс. – NimChimpsky

ответ

3

Вот некоторые улучшения, которые я хотел бы предложить для вашего кода: -

  1. Во-первых, следовать Java Naming Conventions. Имя переменной должно начинаться с строчных букв. Available ->available, или даже лучше, availableDates, чтобы соответствовать цели вашего List

  2. Всегда используйте родового типа коллекций.

    setAllDatesAvailable(ArrayList Array) 
    

    к:

    setAllDatesAvailable(ArrayList<Integer> array); 
    
  3. Используйте Interface как ваши ссылочных типов. Вы должны объявить список, как: -

    List<Integer> availableDates = new ArrayList<Integer>(); 
    

    То же самое и в формальных параметров в своих методах, как показано в комментарии по @Alex.

  4. Выберите подходящее имя для переменных.

    Car number1 = ...; <-- // makes no sense to reader 
    

    изменить его: -

    Car car1 = ...; 
    
  5. Насколько это возможно, попробуйте назвать свои formal параметры так же, как ваши actual параметров.

    setAllDatesAvailable(ArrayList<Integer> array); 
    

    должен лучше быть объявлен как: -

    setAllDatesAvailable(List<Integer> availableDates); 
    

После всех этих изменений, вы можете смотреть вперед, чтобы импровизировать функциональность вашего кода, который @DNA имеет очень хорошо написано в его answer.

+0

Используйте также интерфейс 'List' для параметров в' setAllDatesAvailable (List availableDates); 'или' Collection', если вы просто проходите его. – Alex

+0

@Alex .. О да. Я пропустил это. –

3

Было бы проще записать доступность в виде массива булевых (или ints). Нет необходимости использовать ArrayList для записей фиксированной длины, подобных этому (основное преимущество ArrayList заключается в том, что он может динамически расти).

Я думаю, что было бы лучше использовать значение по умолчанию для boolean или int для указания доступности - тогда вам не нужно явно инициализировать массив вообще. Для булева это будет означать запись заказов не доступность:

boolean[] booked = new boolean[31]; 
    System.out.println(booked[0]); // false 

Это также выдвигает на первый план возможной путаницы - дни месяца часто запутываются в программном обеспечении, так как нулевой индексации, так что вы должны быть ясно, будет ли 1-й день месяц равен 0 или 1.

Было бы лучше и более объектно ориентировано хранить (и при необходимости инициализировать) заказы/наличие внутри объекта объекта «Автомобиль» и предоставлять методы на объекте «Автомобиль» для управления этим данные по мере необходимости.

+0

+1 для булевого массива, нет необходимости в списке для этого. – Alex

0

Что-то вроде внизу возможно - я бы избегал проходить мимо обнаженных коллекций. LocalDate находится в библиотеке joda. Объем по умолчанию на Автомобиле преднамерен и предполагает, что они находятся в одном пакете, но клиент не является:

public class Showroom { 

    private Collection<Car> cars = new ArrayList<Car>(); 

    public boolean isCarAvailable(Car car, LocalDate day) { 
     return car.isAvailable(day); 
    } 

    public void bookCarOnDate(Car car, LocalDate day) throws IlegalStateException { 
      car.book(day); 
    } 

    public Collection<Car> getCars() { 
      return Collection.unmodifiableCollection(cars); 
    } 
} 

class Car { 

    private Map<LocalDate, Boolean> dayAvailability = new HashMap<LocalDate, Boolean>(); 

    Car() { 
     //add other private attributes 
     //don't provide setters for these - create new whenever modified so don't leak 
     //state and thread safety 
    } 

    boolean isAvailable(LocalDate day) { 
     return dayAvailability.get(day) != null && dayAvailability.get(day); 
    } 

    void book(LocalDate day) { 
    if (isAvailable(day)) { 
     dayAvailability.put(car, true); 
    else { 
     throw IllegalStateException("Car "+car+" not available for booking on date "+localDate); 
    } 
} 
Смежные вопросы