6
\$\begingroup\$

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<String, List<Integer>> lakeWood = new HashMap<>();
    private HashMap<String, List<Integer>> bridgeWood = new HashMap<>();
    private HashMap<String, List<Integer>> ridgeWood = new HashMap<>();
    private static int lakeWoodRating;
    private static int bridgeWoodRating;
    private static int rigdeWoodRating;

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

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

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

        lakeWood.put("Rewards", lakeWoodRewards);
        lakeWood.put("Regular", lakeWoodRegular);
        lakeWoodRating = 3;
    }

    private void setBridgeWood() {
        List<Integer> bridgeWoodRewards = new ArrayList<>();
        bridgeWoodRewards.add(110);
        bridgeWoodRewards.add(50);

        List<Integer> bridgeWoodRegular = new ArrayList<>();
        bridgeWoodRegular.add(160);
        bridgeWoodRegular.add(60);

        bridgeWood.put("Rewards", bridgeWoodRewards);
        bridgeWood.put("Regular", bridgeWoodRegular);
        bridgeWoodRating = 4;
    }

    private void setRidgeWood() {
        List<Integer> ridgeWoodRewards = new ArrayList<>();
        ridgeWoodRewards.add(100);
        ridgeWoodRewards.add(40);

        List<Integer> ridgeWoodRegular = new ArrayList<>();
        ridgeWoodRegular.add(220);
        ridgeWoodRegular.add(150);

        ridgeWood.put("Rewards", ridgeWoodRewards);
        ridgeWood.put("Regular", ridgeWoodRegular);
        rigdeWoodRating = 5;
    }

    public String minCostHotel(String input) {
        // Regular: 16Mar2009(mon), 17Mar2009(tue), 18Mar2009(wed)
        String[] arr = input.split(",");
        String customerType = "";
        if (arr.length > 0) {
            String tempArr[] = arr[0].split(":");
            customerType = tempArr[0];
            arr[0] = tempArr[1];
        }
        String hotel = minCostHotel(customerType, arr);
        return hotel;
    }

    private String minCostHotel(String customerType, String[] arr) {
        int numOfWeekdays = 0;
        int numOfWeekends = 0;
        int lakeWoodCost = 0;
        int bridgeWoodCost = 0;
        int ridgeWoodCost = 0;
        for (int i = 0; i < arr.length; i++) {
            String day = arr[i];
            System.out.println(day);
            if (day.contains("mon") || day.contains("tue") || day.contains("wed") || day.contains("thu") 
                    || day.contains("fri")) {
                numOfWeekdays++;
            } else {
                numOfWeekends++;
            }
        }
        List<Integer> valuesForLakewood = lakeWood.get(customerType); // 0 - weekday, 1- weekend
        List<Integer> valuesForBridgeWood = bridgeWood.get(customerType);
        List<Integer> valuesForRidgeWood = ridgeWood.get(customerType);
        lakeWoodCost = numOfWeekdays * valuesForLakewood.get(0) + numOfWeekends * valuesForLakewood.get(1);
        bridgeWoodCost = numOfWeekdays * valuesForBridgeWood.get(0) + numOfWeekends * valuesForBridgeWood.get(1);
        ridgeWoodCost = numOfWeekdays * valuesForRidgeWood.get(0) + numOfWeekends * valuesForRidgeWood.get(1);
        String hotel = minCost(lakeWoodCost, bridgeWoodCost, ridgeWoodCost);
        System.out.println(lakeWoodCost + " " + bridgeWoodCost + " " + ridgeWoodCost);
        return hotel;
    }

    private String minCost(int lakeWoodCost, int bridgeWoodCost, int ridgeWoodCost) {
        int minCost = Math.min(lakeWoodCost, Math.min(bridgeWoodCost, ridgeWoodCost));
        if (minCost == lakeWoodCost && minCost == bridgeWoodCost) {
            return bridgeWoodRating > lakeWoodRating ? "BridgeWood" : "LakeWood";
        } else if (minCost == bridgeWoodCost && minCost == ridgeWoodCost) {
            return bridgeWoodRating > ridgeWoodCost ? "BridgeWood" : "RidgeWood";
        } else if (minCost == lakeWoodCost && minCost == ridgeWoodCost) {
            return lakeWoodCost > ridgeWoodCost ? "LakeWood" : "RidgeWood";
        } else {
            if (minCost == lakeWoodCost) {
                return "LakeWood";
            } else if (minCost == bridgeWoodCost) {
                return "BridgeWood";
            } else {
                return "RidgeWood";
            }
        }
    }

    public static void main(String[] args) {
        // TODO Auto-generated method stub
        //String input = "Regular: 16Mar2009(mon), 17Mar2009(tue), 18Mar2009(wed)"; //-> LakeWood
        String input = "Rewards: 26Mar2009(thu), 27Mar2009(fri), 28Mar2009(sat)"; // -> RidgeWood
        HotelRoomReservation hotelRoom = new HotelRoomReservation();
        String hotel = hotelRoom.minCostHotel(input);
        System.out.println(hotel);
    }

}

Unfortunately, I was rejected even after successfully executing and running the code and after getting the expected results. Can anyone please review this code and tell me what mistakes I have done that could have potentially led to my rejection?

\$\endgroup\$
  • \$\begingroup\$ Can you expand on what your program should do? Eg what are rewards and regular? \$\endgroup\$ – tim Apr 29 '17 at 20:13
  • \$\begingroup\$ Edited the question. Check it. \$\endgroup\$ – Newbie Apr 29 '17 at 21:08
  • 5
    \$\begingroup\$ The major flaw is that you've hard-coded the various alternatives. A better solution would be to make it dynamic and expandable, so that it would be easy to add new hotels. Imagine your code with a hotel chain of a 100 hotels... \$\endgroup\$ – holroy Apr 29 '17 at 21:28
  • 1
    \$\begingroup\$ I wrote a blog post a little while ago, that solves this specific problem with OOP... in VBA, but might still give you ideas: rubberduckvba.wordpress.com/2016/07/05/… \$\endgroup\$ – Mathieu Guindon Apr 29 '17 at 22:41
  • 3
    \$\begingroup\$ Reading the problem statement should trigger thoughts about the involved objects and data: you know you need a concept of customer type and a way to tell weekday vs weekend rates; seeing 3 hotels with different data is a hint that you need some Hotel class, and 3 instances of it: you don't want to constraint your code to just these 3 hotels, so you extract and parameterize as much as you can. ..it's just a thought process really. \$\endgroup\$ – Mathieu Guindon Apr 30 '17 at 3:42
5
\$\begingroup\$

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<String, List<Integer>> lakeWood = new HashMap<>();
    private HashMap<String, List<Integer>> bridgeWood = new HashMap<>();
    private HashMap<String, List<Integer>> ridgeWood = new HashMap<>();

It would be more common to write this as

    private Map<String, List<Integer>> lakeWood = new HashMap<>();
    private Map<String, List<Integer>> bridgeWood = new HashMap<>();
    private Map<String, List<Integer>> 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 < minimum || (cost == minimum && hotel.getRating() > 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.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.