0
\$\begingroup\$

I am writing Vehicle - Components - Car relationship. I have to separate cars that use the diesel, petrol and electric engines because each of them produce their own results (but methods of each are the same).

For example, if car is using diesel engine (which obviously should have only diesel exhaust system) then it produces higher pollution than electric car's engine.

It is very important that after I could set the engine, for example my Car has diesel engine and now I want to use electric instead because of lower pollution.

Right now, I have an abstract class Vehicle. Vehicle has-a components such as Engine and Tires. Car is-a (extends) Vehicle.

Vehicle.class

public abstract class Vehicle {
    private Engine engine;
    private Tire tires;

    public Engine getEngine() {
        return engine;
    }

    public void setEngine(Engine engine) {
        this.engine = engine;
    }

    public Tire getTires() {
        return tires;
    }

    public void setTires(Tire tires) {
        this.tires = tires;
    }

    public MotorVehicle() {
        super();
    }

    public Vehicle(Engine engine, Tire tires) {
        this.engine = engine;
        this.tires = tires;
    }
}

Car.class

public class Car extends Vehicle {
    private Engine engine;
    private Tire tires;
    private ExhaustSystem system;

    public Car() {
        super();
    }

    public Car(Engine engine, Tire tires) {
        super(engine, tires);
    }
}

Where Engine.class is an interface

public interface Engine {
    String getType();
    ExhaustSystem getExhaustSystem();
}

and, for example, DieselEngine.class implements it

public class DieselEngine implements Engine {
    private String type;
    private DieselExhaustSystem system;

    public DieselEngine(String type, DieselExhaustSystem system) {
        this.type = type;
        this.system = system;
    }

    @Override
    public String getType() {
        return type;
    }
    @Override
    public ExhaustSystem getExhaustSystem() {
        return system;
    }
}

Similar to Engine - ExhaustSystem.class is an interface

public interface ExhaustSystem {
    double getGasExits();
}

and DieselExhaustSystem.class implements it.

public class DieselExhaustSystem implements ExhaustSystem {

    /**
     *
     * @return CO2 emission units.
     */
    @Override
    public double getGasExits() {
        return 3;
    }
}

I also will have Truck which will extend Vehicle.

After I wrote this, I have some doubts about its correctness.

The main question is:

  1. Is relationship and classes logic correct?

If not or partially not, then:

  1. What can be done better?
  2. Maybe I do not understand polymorphism?
  3. Maybe I do not understand has-a / is-a relationships?
  4. Maybe I should use another desing pattern for my task (for example, bridge pattern)?
  5. Perhaps it is better to make Engine abstract and other engines extend it?
\$\endgroup\$

closed as off-topic by vnp, Sᴀᴍ Onᴇᴌᴀ, Ludisposed, t3chb0t, Toby Speight Jan 8 '18 at 14:54

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – vnp, Sᴀᴍ Onᴇᴌᴀ, Ludisposed, t3chb0t
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Mostly it looks OK to me. One thing that looks like a mistake to me is to have both Vehicle and Car declare an engine field. This will give all cars 2 engines: one from Vehicle and one from Car. Probably not what you want. Use only one field (probably in Vehicle, unless some vehicles you will implement do not have engines (sail boat?)). \$\endgroup\$ – markspace Jan 6 '18 at 22:15
  • \$\begingroup\$ (Clearly, a jet-ski has a motor, hybrids (Diesel-electric submarines, anyone?) diverse per definition, gliders & dumb barge none: maybe vehicle should have a bag of motors. It's a pity that object-oriented doesn't mention modelling.) \$\endgroup\$ – greybeard Jan 7 '18 at 1:32
1
\$\begingroup\$

Thanks for sharing your code.


OOP doesn't mean to "split up" code into random classes.

The ultimate goal of OOP is to reduce code duplication, improve readability and support reuse as well as extending the code.

Doing OOP means that you follow certain principles which are (among others):

  • information hiding / encapsulation
  • single responsibility
  • separation of concerns
  • KISS (Keep it simple (and) stupid.)
  • DRY (Don't repeat yourself.)
  • "Tell! Don't ask."
  • Law of demeter ("Don't talk to strangers!")

We create a new class if we need to implement new behavior not yet present nor belonging to any existing class.

In your example the class Car does not (yet) add any new behavior neither to the class Vehicle which it extends nor to the program as a whole. Therefore it is not needed (as well as your planned class Truck) and Vehicle should not be abstract. If you need to identify different vehicle types you might introduce a vehicleType property of type String...

\$\endgroup\$
  • 1
    \$\begingroup\$ I would say better use a vehicleType property of type enum \$\endgroup\$ – Sharon Ben Asher Jan 8 '18 at 9:51
  • \$\begingroup\$ @SharonBenAsher: It depends: at the moment the vehicle type is just a display name without any semantical meaning. Therefore I'd stick with the String for the moment. \$\endgroup\$ – Timothy Truckle Jan 8 '18 at 16:44

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