"Find Cheapest Hotel" interview

I was asked to write code to get the cheapest hotel to stay based on the input. There are three hotels with different weekend and weekday rates and rating. So when the input string is passed with the customer type and the dates, I had to return the cheapest hotel based on the rates and if there is a tie return hotel with highest rating.


A hotel chain wishes to offer room reservation services over the internet. They have three hotels : Lakewood, Bridgewood and Ridgewood. Each hotel has separate weekday and weekend (Saturday and Sunday) rates. There are special rates for rewards customer as a part of loyalty program. Each hotel has a rating assigned to it.



-
Lakewood with a rating of 3 has weekday rates as 110$ for regular customer and 80$ for rewards customer. The weekend rates are 90$ for regular customer and 80$ for a rewards customer.

-
Bridgewood with a rating of 4 has weekday rates as 160$ for regular customer and 110$ for rewards customer. The weekend rates are 60$ for regular customer and 50$ for a rewards customer.

-
Ridgewood with a rating of 5 has weekday rates as 220$ for regular customer and 100$ for rewards customer. The weekend rates are 150$ for regular customer and 40$ for a rewards customer.


Write a program to help an online customer find the cheapest hotel.

```
import java.util.*;
public class HotelRoomReservation {
private HashMap> lakeWood = new HashMap<>();
private HashMap> bridgeWood = new HashMap<>();
private HashMap> ridgeWood = new HashMap<>();
private static int lakeWoodRating;
private static int bridgeWoodRating;
private static int rigdeWoodRating;

public HotelRoomReservation() {
setLakeWood();
setBridgeWood();
setRidgeWood();
}

private void setLakeWood() {
List lakeWoodRewards = new ArrayList<>();
lakeWoodRewards.add(80);
lakeWoodRewards.add(80);

List lakeWoodRegular = new ArrayList<>();
lakeWoodRegular.add(110)

Individual imports

import java.util.*;

Most IDEs will manage individual imports for you. So you don't need to use wildcard imports.

Match names to purpose

public class HotelRoomReservation {

But this class does not represent a reservation. It books reservations.

public class HotelBooker {

That would leave HotelRoomReservation free to be used elsewhere, where it is more descriptive.

Interface types

private HashMap> lakeWood = new HashMap<>();
    private HashMap> bridgeWood = new HashMap<>();
    private HashMap> ridgeWood = new HashMap<>();

It would be more common to write this as

private Map> lakeWood = new HashMap<>();
    private Map> bridgeWood = new HashMap<>();
    private Map> ridgeWood = new HashMap<>();

This way, if we want to change the implementation, we can do so at just one place.

Initialize static variables in static context

private static int lakeWoodRating;

and later called from the constructor

lakeWoodRating = 3;

Why make this static if you are just going to set it at construction time?

You could just say

private static int lakeWoodRating = 3;

Then it would be set once, statically.

You also might consider replacing the existing object methods with static class methods called from a static initializer block rather than a constructor.

Avoid magic indexes

lakeWoodRegular.add(110);
        lakeWoodRegular.add(90);

How do you know which is the weekday rate and which the weekend? You have to manually keep track. This is fragile and hard to code, especially since you don't comment this at all. Figuring out how to add a new hotel would be difficult and subject to error.

Use enum keys

lakeWood.put("Rewards", lakeWoodRewards);
        lakeWood.put("Regular", lakeWoodRegular);

You are using strings as constant values here. Consider

lakeWood.put(CustomerType.REWARDS, lakeWoodRewards);
        lakeWood.put(CustomerType.REGULAR, lakeWoodRegular);

Then a typo will show up in compilation. With the strings, typos won't show until runtime. Further, they will cause incorrect results far from the typo. Using enum keys also allows a special, extra-efficient form of Map: the EnumMap.

Methods get verb names

public String minCostHotel(String input) {

This is a noun. I would expect a method to be named something like

public String findMinimumCostHotel(String input) {

Although to tell the truth, most of this method is about parsing the request. It's only the last two lines that have anything to do with processing the request. Consider separating request parsing from request processing. As is, the other minCostHotel does both.

Don't submit debugging code

System.out.println(lakeWoodCost + " " + bridgeWoodCost + " " + ridgeWoodCost);

This would seem to be just for debugging. It should not have been submitted to the evaluator (nor for review here).

// TODO Auto-generated method stub

This also should go prior to submission. It exists to remind you that you have work to do. Once you do that work, delete this. And you certainly don't submit it. The person reviewing your code doesn't need to be reminded that you had work to do.

Write code that scales

In general, this would be more robust if you replaced your manual if/else structure with something that naturally expands. E.g.

public String findCheapestHotelName(ReservationRequest request) {
    Hotel cheapest = hotels.get(0);
    int minimum = choice.calculateCost(request);
    for (Hotel hotel : hotels.subList(1, hotels.size())) {
        int cost = hotel.calculateCost(request);

        if (cost  choice.getRating())) {
            cheapest = hotel;
            minimum = cost;
        }
    }

    return cheapest.getName();
}

Now adding a new hotel is as simple as adding an entry to the list.

Try adding a fourth hotel to your original code to compare.

StackExchange Code Review Q#162138, answer score: 5