Parking places design

Question

I got a question:

Design a parking system. There are 3 types of parking spaces: big, medium and small.

Implement ParkingSystem class(int big, int medium, int small). Constructor takes a number of places of types. So ParkingSystem(1,2,3) means there are 1 big place, 2 medium and 3 small.

Add method addCar(int carType) - carType takes one of three values [1,2,3] which represents big car, medium car and small car.

Method should check if there is a place for a car of such size or bigger, if yes then it parks the car in this place and returns true, else false.

For now there is no method to free parking space (when a car is leaving) but it can be added in future.

Example:

• ParkingSystem ps = new ParkingSystem(1, 2, 3); // creates parking with 1 big place, 2 medium and 3 small
• ps.addCar(2); //true, medium car takes medium place
• ps.addCar(2); //true, medium car takes medium place
• ps.addCar(2); //true, medium car takes big place
• ps.addCar(1); //false, no place for big car

I wrote:

public class ParkingSystem {
    private int big;
    private int medium;
    private int small;

    public ParkingSystem(int big, int medium, int small) {
        this.big = big;
        this.medium = medium;
        this.small = small;
    }

    public boolean addCar(int car) {
        if (car == 1 && isPlaceForBigCar()) {
            return parkBigCar();
        } else if (car == 2 && isPlaceForMediumCar()) {
            return parkMediumCar();
        } else if (car == 3 && isPlaceForSmallCar()) {
            return parkSmallCar();
        }
        return false;
    }

    private boolean parkBigCar() {
        this.big--;
        return true;
    }

    private boolean parkMediumCar() {
        if (medium > 0) {
            this.medium--;
        } else if (big > 0) {
            this.big--;
        }
        return true;
    }

    private boolean parkSmallCar() {
        if (small > 0) {
            this.small--;
        } else if (medium > 0) {
            this.medium--;
        } else if (big > 0) {
            this.big--;
        }
        return true;
    }

    private boolean isPlaceForBigCar() {
        return this.big > 0;
    }

    private boolean isPlaceForMediumCar() {
        return (this.big > 0 || this.medium > 0);
    }

    private boolean isPlaceForSmallCar() {
        return (this.big > 0 || this.medium > 0 || this.small > 0);
    }
}

so it seems to be a little better than the worst solution but I believe there is still a lot to improve. Should I use strategy pattern to park cars or is there even better solution?

Main/Test class:

public class Main {
    public static void main(String[] args) {
        ParkingSystem ps = new ParkingSystem(1, 2, 3); // creates parking with 1 big place, 2 medium and 3 small
        System.out.println(ps.addCar(2)); //true, medium car takes medium place
        System.out.println(ps.addCar(2)); //true, medium car takes medium place
        System.out.println(ps.addCar(2)); //true, medium car takes big place
        System.out.println(ps.addCar(1)); //false, no place for big car
    }
}

Answer 1

this.

There's a lot of this. references in your code. Typically in Java you only use this. where you need to in order to disambiguate variables. Consider removing them as they're only adding noise to your code.

always True

Your park methods all return boolean, even though they all always return true. If you want to maintain the same approach, consider changing them to void.

That said, I'd be tempted to merge the isPlaceFor and park logic together into a single tryPark method that checks if it can park and parks it if it can. This seems like a connected operation. Are you ever likely to want to know if you can park there without actually parking the car?

Naming / Constants

You fields are big,medium,small. You use them to track the available spaces of that size, consider being a bit more verbose with the field names to make the meaning clearer. It'll help when you add maxAvailableBigSpaces for example.

I'd also consider adding constants for the CAR sizes. Whilst it may be obvious at the moment that 3 is a small car, it may not be as obvious in a week/months time.

delegating

When you park a small car, you're effectively checking for the small space, and if it's not there, trying to park it as a medium car, which if it's not possible you then try to park as a large car. You could match this approach in your code, which simplifies the logic in each stage and removes some of your duplicate checks.

switch

Combining some of the above together and using a switch expression rather than the if/elseif/else combo means that you could end up with some code more like this:

public class ParkingSystem {
    private static final int BIG_CAR = 1;
    private static final int MEDIUM_CAR = 2;
    private static final int SMALL_CAR = 3;
    private int availableBigSpaces;
    private int availableMediumSpaces;
    private int availableSmallSpaces;
    
    public ParkingSystem(int bigSpaces, int mediumSpaces, int smallSpaces) {
        availableBigSpaces = bigSpaces;
        availableMediumSpaces = mediumSpaces;
        availableSmallSpaces = smallSpaces;
    }

    public boolean addCar(int car) {
        return switch (car) {
            case BIG_CAR -> tryParkBig();
            case MEDIUM_CAR -> tryParkMedium();
            case SMALL_CAR -> tryParkSmall();
            default -> false;
        };
    }

    private boolean tryParkBig() {
        if(availableBigSpaces > 0) {
            availableBigSpaces--;
            return true;
        }
        return false;
    }

    private boolean tryParkMedium() {
        if (availableMediumSpaces > 0) {
            availableMediumSpaces--;
            return true;
        }
        return tryParkBig();
    }

    private boolean tryParkSmall() {
        if (availableSmallSpaces > 0) {
            availableSmallSpaces--;
            return true;
        }
        return tryParkMedium();
    }
}

A simple array approach

An alternate approach, which is more concise, however I don't think is as clear to follow, would be to use the ordering of the car sizes and an array to handle the allocation.

public class ParkingSystem {
    private int availableSpaces[] = new int[3];

    public ParkingSystem(int bigSpaces, int mediumSpaces, int smallSpaces) {
        availableSpaces[2]=smallSpaces;
        availableSpaces[1]=mediumSpaces;
        availableSpaces[0]=bigSpaces;
    }

    public boolean addCar(int car) {
        if(car > 3) return false;
        for(var spaceToCheck = car-1;spaceToCheck>=0;spaceToCheck--) {
            if(availableSpaces[spaceToCheck] > 0) {
                availableSpaces[spaceToCheck]--;
                return true;
            }
        }
        return false;
    }
}

Answer 2

Always True

In addition to forsvarir's answer, especially the "always True" aspect, I'd like to give an additional reason why to change that.

Your parkBigCar() method (and its siblings) is only legal to be called after isPlaceForBigCar() has been checked and returned true. You surely know that today and wrote all your calls appropriately, but if you're working in a professional team, on a project lasting e.g. a year, it's quite possible that you (or your colleague) have to change something in that class at some time in the future, and have forgotten (or never known) about that fact.

The more robust approach to such a situation is to design your methods in such a way that they can legally be called in any order you can imagine. And you already prepared to do so by declaring a boolean return value, most probably with the idea in mind to return false and not change anything about the parking lot if no space is available. That's completely in line with forsvarir's suggestion.

If you don't integrate that check into the methods, at least document the dependency (and I'd recommend using Javadoc syntax here):

/**
 * Park a big car.
 * This method assumes that space is available.
 * Before calling this methods, the caller must check with isPlaceForBigCar().
 */
private boolean parkBigCar() {
    this.big--;
    return true;
}

By the way, it's always a good idea to document (at least for public methods) what their task is, or even better to define a method's task before you implement it.

But let me as well mention the many good aspects of your solution:

• Your names are descriptive and follow the established Java Naming Guidelines.
• You decisions what to make private or public are perfect.
• Your code is straightforward and thus easily understood.

Strategy Pattern

Finally, your question about the Strategy Pattern has not yet been answered.

You'll find lots of patterns on the web, and most of them have their applications where they solve some problems, but I'd recommend to take all the pattern appraisals with a grain of salt. If the problem at hand is not the one the pattern is meant for, it won't improve anything.

On the other hand, applying some patterns can make code more complex than the straightforward solution, and that's what we call over-engineering.

In the industry, code reliability and code readability are very important qualities, so if a pattern doesn't make your code more reliable or more readable in a given situation, don't apply it.

Returning to the Strategy Pattern: there's no way to improve the assignment of cars to parking places, so (citing Wikipedia) "...that enables selecting an algorithm at runtime" doesn't make any sense in your situation. You got the perfect algorithm, no need to change that, especially not to select multiple exchangable algorithms at runtime.

So, introducing the Strategy Pattern would be over-engineering, and a typical case of YAGNI.