3

I had an interview yesterday and had asked an OOD question:

Race-Car Store System:

The system stores information about cars available for players.

  • Two types of gear changing strategies: manual/automatic.
  • Two types of fuel: gasoline/diesel.

Design a system that can produce cars requested by players (If the player wants a car with manual gear changing and burn diesel, your system should provide one instance of the car meets the requirements). The system should have good scalability and maintainability.

My thoughts and solution:

My thought is the requirement contains two attributes: gear and fuel. I plan to make an abstract class contains the attributes and corresponding behaviors. Considering scalability would have an interface Movable which contains behaviors what a car can do.

If any new attribute is added in the future, either create a new abstract class contains the new attribute or add the attribute into the existing abstract class, if there's new behaviors are required, I would either create new interface or add the behavior into existing interface.

Here's what I have done: An interface contains general behaviors, currently has showSpecs() only.

public interface Movable { public String showSpecs(); }

An abstract class contains attributes fuel and gear

public abstract class Car implements Movable { String gear; String fuel; abstract void setFuel(String fuel); abstract String getFuel(); abstract void setGear(String gear); abstract String getGear(); }

Now the race car class implementaion:

public class RaceCar extends Car { public RaceCar(String fuel, String gear) { this.fuel = fuel; this.gear = gear; } public void setFuel(String fuel) { this.fuel = fuel; } public String getFuel() { return this.fuel; } public void setGear(String gear) { this.gear = gear; } public String getGear() { return this.gear; } public String showSpecs() { StringBuilder sb = new StringBuilder(); sb.append("Gear:").append(this.gear); sb.append("Fuel:").append(this.fuel); return sb.toString(); } }

Below is the main class I have:

public class Main { public static void main(String[] args) { System.out.println("get started..."); Car car = new RaceCar("diseal", "automatic"); System.out.println(car.showSpecs()); } }

The interviewer replied that the solution I provided is not scalable and hard to maintain but didn't provide details so I am still confused about what mistakes I made and how to improve it.

Can anyone help share your thoughts and point out what am I supposed to improve?

Thanks!

Improve this question
6
  • I would perhaps have pressed him for a slightly better-defined requirement than "good". Commented Jul 24, 2020 at 20:36
  • 4
    You've over-complicated this with respect to the requirements, because there doesn't seem to be a need for Movable or Car: just define a RaceCar class; give it two fields; give it a constructor. Define enums for the fuel and car fields' values. But scalability - I struggle to understand what is actually being asked for there: you invoke the constructor each time you want an instance; just invoking the constructor seems necessary and sufficient. Commented Jul 24, 2020 at 20:41
  • 1
    I don't like that you use normal Strings for the attributes. Better use enums (or something else, which depends on how those instances are actually used). And I don't like the requirements much because they don't tell much about how those instances are used. This makes it difficult to design something that is not just generic. Maybe they expected the factory pattern or strategy pattern. Who knows .. Commented Jul 24, 2020 at 20:44
  • @AndyTurner Thanks Andy for your reply. I have asked what kind of scalable is expected and got answer "add tank". I didn't mention the details as one of the interviewers saw the source code, he said it's not an answer he expected. Commented Jul 24, 2020 at 20:44
  • The question was ambiguous to me and I asked a couple of questions about it but didn't get a clear reply. I thought it probably open because "scalability" and "maintainable" are too wide for me. Commented Jul 24, 2020 at 20:46

2 Answers 2

Reset to default
1

I would have answered this question with 2 classes, Car and CarBuilder:

public final class Car { private final Fuel fuel; private final Gears gears; public Car(Fuel fuel, Gears gears) { this.fuel = fuel; this.gears = gears; } public Fuel getFuel() { return fuel; } public Gears getGears() { return gears; } enum Fuel { GASOLINE, DEISEL } enum Gears { AUTOMATIC, MANUAL } } public class CarBuilder { //sensible defaults: private Car.Fuel fuel = Car.Fuel.GASOLINE; private Car.Gears gears = Car.Gears.MANUAL; public CarBuilder() { } public CarBuilder withFuelType(Car.Fuel fuel) { this.fuel = fuel; return this; } public CarBuilder withGearBox(Car.Gears gears) { this.gears = gears; return this; } public Car build() { return new Car(this.fuel, this.gears); } }

Scalability and maintainability is achieved by the fact that these are the only 2 classes that need to be changed in the future, should requirements change. Car is immutable and also contains the enums required to represent its internal state, so these attributes can't leak out of the context/object in which they make sense so make it easier to maintain in the future.

The builder class, while basic in its current form, can be extended to accommodate more complex construction requirements without leaking implementation details into the Car class.

The default values are optional, but might make sense.

A car can be constructed as such:

//Default car: Car car = new CarBuilder().build(); //Customised car: Car car = new CarBuilder().withFuelType(Car.Fuel.DEISEL).withGearBox(Car.Gears.AUTOMATIC).build();
Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

Thank you @StuPointerException for providing an answer, it helps a lot, +1!
What's the point of having private fields, but then having accessor methods to those fields? One doesn't need an advanced degree in graph theory to see that public fields and accessor methods achieve the same effective outcome. Also, you don't need to CarBuilder object. You can simply call new Car(this.fuel, this.gears).
@AboveTheGods they do have the same effective outcome, but hiding the private fields can offer additional benefits (see stackoverflow.com/questions/11071407/… for some good points). You're right about the Constructor of Car, I should have made that private so the only place a car can be constructed is within the builder, this encapsulates the building logic and ensures that the car can only be built to valid specifications.
1

I thought that maybe he was expecting something like pluggable classes when he mentioned scalable and maintainable. So I think maybe this strategy pattern was expected. If transmission or injection is expected to do some real logic, I can assume them as behaviors instead of just state. Thus results with this implementation.

public interface TransmissionPolicy { public void transmit(); } public class AutomaticTransmission implements TransmissionPolicy { public void transmit() { //do some real logic here print("automatic..."); } } public class ManualTransmission implements TransmissionPolicy { public void transmit() { print("we love it..."); //just an example of really simple logic } } public interface InjectionPolicy { public void inject(); } public class DieselInjection implements InjectionPolicy { public void inject() { print("diesel"); } } public class GasolineInjection implements InjectionPolicy { public void inject() { print("gasoline..."); } } public class Car { public void make(TransmissionPolicy transmission, InjectionPolicy injection) { //set other parts transmission.transmit(); //set other parts injection.inject(); //other parts } } //--------------somewhere in some clients client -------------------- Car car = new Car(); //actually, to be really configurable use a factory method here. car.make(new ManualTransmission(), new GasolineInjection());

If that was expected then with just lambdas or command pattern it would be made also.

Improve this answer

2 Comments

Thanks Taha for answering, +1 !
There is no point at all for most of the types here. There is no logic for TransmissionPolicy! It's not specified in the requirements.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.