Video rental store application design

I need some help in working "Video Rental Store" application design optimization. The application describes the following tasks:

• Inventory stores videos. Inventory can add/delete/change and receive all videos. Inventory has 3 types of films: Old, New, Regular.

• Videos have a title, type (described above) and availability which will be changed to 'not available'/'available' if this video is rented by someone.

• Customer can rent only available videos(not rented by someone) with additional option to rent film for bonus points which gathers every time when a customer rents a video (1 point for rent)

• Before the customer rents a video he/she set renting days. The price for rent calculated automatically based on rent days, video type. After the price is calculated for this rent transaction than customer is presented terms where he/she has a choice to accept or reject them. If a customer accepts terms than rent transaction is executed successfully.

Data is not persisted anywhere, data exists in runtime. Inventory and RentalStore class generates some fake records for presentation.

Could you please review my app and offer some tips or optimization tricks?

In general, I'm not sure in RentalStore class which implements connections to the Inventory and RentalTransactions classes. I don't know how to implement elegant class which will work with rental transactions and will know what items have inventory. I read some principles of design patterns how to organize application that it will be flexible as much as possible but it seems I need more experience in design area. That is why I contact with you. Someone suggested me that it is possible to do it better than my way.

RentalStore class:

```
public interface RentalDAO {
void rentItem(Person person, Item item, int days, Boolean temsAcceptance);

List getAllRentals();

List getAllRentalsForCustomer(Person person);
}

public class RentalStore implements RentalDAO {

private Inventory inventory;
private Lis

I think you are getting lost in a bunch of methods you are implementing before you have a real justification for them. So let's just ignore those for the moment, and concentrate on the bits that you have a handle on (in the expectation that things will become clearer).

private double calculate() {
    switch (item.getType()) {
        case NEW_RELEASES:
            this.price = PREMIUM_FEE * days;
            break;
        case REGULAR:
            if (days > 3) {
                this.price = REGULAR_FEE + REGULAR_FEE * (days - 3);
            } else {
                this.price = REGULAR_FEE * days;
            }
            break;
        case OLD:
            if (days > 5) {
                this.price = REGULAR_FEE + REGULAR_FEE * (days - 5);
            } else {
                this.price = REGULAR_FEE * days;
            }
            break;
    }
    return this.price;
}

First code smell - you are using a switch to change the behavior of your program. That usually means there's an abstraction that you are missing, often something that uses the StrategyPattern, and some sort of selector.

private double calculate() {
    PricingPolicy pricingPolicy = getPricingPolicy(item);
    return pricingPolicy.getPrice(days);
}

class PerDiemPricingPolicy implements PricingPolicy {
    private final double perDiem;

    double getPrice(int rentalDays) {
      return perDiem * rentalDays;
    }
}

static final PricingPolicy DEFAULT_PRICING_POLICY = new PerDiemPricingPolicy(REGULAR_FEE); 
static final PricingPolicy NEW_RELEASE_PRICING = new PerDiemPricingPolicy(PREMIUM_FEE);

class DiscountedPricingPolicy implements PricingPolicy {
    private final int discountPeriod;
    private final PricingPolicy pricingPolicy;

    double getPrice(int days) {
        if (days > discountPeriod) {
            days -= discountPeriod - 1;
        }
        return pricingPolicy(days);
    }
}

static final PricingPolicy REGULAR_PRICING = new DiscountedPricingPolicy(3,DEFAULT_PRICING_POLICY);
static final PricingPolicy OLD_PRICING = new DiscountedPricingPolicy(5, DEFAULT_PRICING_POLICY);

Now getPricingPolicy(Item) might be implemented in any of a number of ways; it could be the switch again, it could be a Map, it could be a Guava Function, it could be a state machine.

At the moment, your policies are all determined by Type (which should probably be an extensible enum), so you know that there needs to be some way to convert Item to Type -- but that doesn't necessarily mean that Type is a property of Item. Again, a guava Function is a good way to insulate your design against a change here.

I think your inventory class is a mess, because you haven't actually tried to use it. My suggestion would be to write out a test case that rents an item to a person. Forget about crud, forget about DAO, just write "Lauri rents Mortal Kombat", and see if Manager Bob can figure out what happened from the TPS report.

StackExchange Code Review Q#52007, answer score: 5