Is this bad oop design?

Question

Imagine I have an interface called IVehicle.

From this interface, I derive several concrete types such as bus and car (all can move, slow down, switch off engine, etc). My interface has no fields.

Would it be bad design to have one class which has fields (e.g. top speed of vehicle) and use this by each of the concrete types? Would this be bad design? Also, if I store state in this class (e.g. use the fields), then would it have to be static?

Thanks

Answer 1

You're talking about an abstract class, and no, it's not bad design. That's what they're for (basically, interfaces, but with some basic implementation).

State wouldn't have to be stored statically. There will be a full implementation of this class (in the form of one of the derived classes) each time you create a Car, Bus, etc.

Answer 2

Just to clear up some confusion...

An interface cannot have fields.

However, it can have properties. Properties that would invariably have to be implemented through code by the objects that implement the interface.

Unless you plan on casting a reference to an object through an interface back to a concrete type, you should put some properties that makes sense into the interface, otherwise you won't be able to reach them.

In other words, if you have this:

IVehicle vehicle = GetVehicle();
// want to get speed of vehicle?
Car car = (Car)vehicle;
Double speed = car.Speed;

Common properties should be put into the common interface. Since a vehicle is a moving object, speed would make sense to have there.

However, the number of engines would make sense for planes and some boats, but not for cars, and such that type of information would probably have to be located into some more specific interfaces.

Static would mean shared between instances, I doubt that would be a good idea.

However, the overall plan to use interfaces instead of just a class hierarchy to separate out the access to and use of an object from its actual type and identity is a good idea, and leads to good design.

The best plan is to pick the level of abstraction that makes sense for your model and go with that. You can over-design such a solution very easily. For instance, you could realize that some vehicles doesn't have engines at all, and some can move in more than one dimension (ie. not just forward/backward, but up/down, sideways, etc.), and picking the level that fits your model is a good start.

For instance, if you're going to model the transportation issues a county has to deal with, whether a plane can move just forward/backward or got vertical rotatable engines or not is moot, so you probably shouldn't try to design that into the solution.

Answer 3

You're being language-specific. In C++, for example, there are no interfaces, but you can use abstract classes like an interface.

Answer 4

I would add TopSpeed to the interface as a property like this:

interface IVehicle
{
    void Move();
    void SlowDown();
    void SwitchOffEngine();

    Int32 TopSpeed { get; }
}

Then you implement the interface in your concrete entities like this:

class Car : IVehicle
{
    public void Move() { }
    public void SlowDown() { }
    public void SwitchOffEngine() { }

    public int TopSpeed
    {
     get { return 120; }
    }
}

class Bus : IVehicle
{
    public void Move() { }
    public void SlowDown() { }
    public void SwitchOffEngine() { }

    public int TopSpeed
    {
     get { return 98; }
    }
}

I think this would be the best approach and doesn't require a base class.

The reason I believe this is the best approach because TopSpeed is a property that is specific to the implementation and will not have any meaning in a base class. Since the base class would require that any implementation set this value anyways I think it is best to change the interface to include it.

Answer 5

I second DannySmurf, abstract classes are very useful. I use the hide-the-abstract-class-behind-an-interface pattern in class libraries very often so that I can have all the implementation classes, even the abstract parent class, as internal classes, that is, I do not expose the implementations to the client at all.

Answer 6

Would it be bad design to have one class which has fields (e.g. top speed of vehicle) and use this by each of the concrete types? Would this be bad design?

No, not bad design. An alternative is to have a abstract / pure virtual property for each field (e.g. top speed) ... if you have a lot of these properties, and if the properties are simple (e.g. just return a number, don't actually do anything complicated) then your idea of a class which contains the specialized property values seems simpler than having that many more abstract / pure virtual properties.

Also, if I store state in this class (e.g. use the fields), then would it have to be static?

It's OK if it's static, if-and-only-if it applies for every instance of the class. For example, it's OK to define "top speed" as static (all busses have the same top speed), but not "current speed" (different busses actually travelling at different speeds at any given moment).

---

Just to be clear, I think you're suggesting a design that's something like this:

interface IVehicle
{
  void accelerate();
}

sealed class VehicleProperties
{
  const int topSpeed;
}

class VehicleImplementation : IVehicle
{
  //const data
  const VehicleProperties vehicleProperties;
  //non-const instance/state data
  int currentSpeed;
  //ctor
  VehicleImplementation(VehicleProperties vehicleProperties)
  {
    this.vehicleProperties = vehicleProperties;
  }
  //implement IVehicle methods
  void accelerate()
  {
    if (currentSpeed => vehicleProperties.topSpeed)
      return;
    ... else todo: accelerate ...
  }
}

class Car : VehicleImplementation
{
  //static properties associated with the Car type
  static VehicleProperties carProperties = new VehicleProperties(120);
  //ctor
  Car()
    : VehicleImplementation(carProperties)
  {}
  //IVehicle methods already implemented by
  //the VehicleImplementation base class
}

Answer 7

You are not breaking OO, in your case-

IVehicle defines "if you are a vehicle what should you be able to do"

AbstractVehicle defines "if you are a vehicle this is the minimum you should be able to do and this is how you would do"

ConcreteVehicle defines " I am a vehicle, this is my way of doing things that I am supposed to be doing since I am a vehicle"

You dont need to put all your properties as static. Something like Max speed or Top speed can be kept static but you could keep them as const too, your call.

Answer 8

I think you should use superclasses and not interfaces. So most likely the IVehicle interface is redundant, and you should just have abstract superclass called Vehicle. As it is true that every vehicle can move and thus can have its speed measured, you can add speed as a field to the superclass.

This speed field is in no way static, because it will be instantiated for every concrete instance of Vehicle; if you have two buses, they may have different speeds, so the field cannot be static but must be specific to the instances.

Answer 9

Another approach, and how I distinguish between abstract classes and interfaces for common scenarios, is:

-All custom behaviour in an interface -Common fields to use in a base class, which can be used and set as required.

I use the hide-the-abstract-class-behind-an-interface pattern in class libraries very often so that I can have all the implementation classes, even the abstract parent class, as internal classes, that is, I do not expose the implementations to the client at all.

Is there an example of this?

Answer 10

If you "add a field to an interface", it stops being an interface and becomes a base class. That might be bad or it might not be, depending on your preferences.

The pattern of using an interface and avoiding base classes with member variable is quite fine. It is especially useful in situations where you pass an interface to another part of a system.

But without a specific context, there is no absolute rule saying "thou must use an interface as one's base class". Using a private member variable in your base class may be a fine first approximation to the functionality you are looking for. Another solution, using a private function defined in the derived class, is more verbose and by that fact alone may make your code harder to understand (though, if everyone in your organization is doing this, that would not be an issue).

It depends on what you would like to accomplish. It also depends on what standard is used in your organization, in your application or even on what language you are using (if you use a language with getters and setters, you can more easily replace member variables at a later time, for example).

Whether a member variable is static or not is a separate issue. You should make a member static if and only it doesn't change over the life of the class. "Top Speed" sounds like it fit this criteria. Some languages, like c++, have the concept of "static functions", whose results don't change. I suppose you could use this as well but I find this added concept too confusing to be worthwhile.

Answer 11

Would it be bad design to have one class which has fields (e.g. top speed of vehicle) and use this by each of the concrete types? Would this be bad design?

No.

IMHO the difficulties with inheritance in C++ (I don't know the case of Java and other languages so well) have helped establish the tradition that "non-leaf classes should be abstract" (it's actually an item of "More Effective C++" by Scott Meyers, if I recall well). But, if anything, putting fields and concrete functionality in a base class is not bad OO design.

In some languages, it may pay off to follow this strategy. In classical OO, and its flag language (Eiffel), this (having concrete, useful base classes if your design leads you to that choice) is exactly the way to go. There are language mechanisms designed to solve any potential problems you could have due to polimorphic use of your code or multiple inheritance.

So, it is not bad OO design; that's for sure. But you should take into account that, in some languages, your natural idea may be traditionally avoided in favour of less natural, but more practical alternatives.

By "your design leads you to that choice" I mean that in your program it makes sense to create instances of "a vehicle". If this is true (for example because you are designing a video game where it is possible to use 'vehicles'), then your design will be right. If in the problem your software is trying to solve it makes no sense whatsoever to create a vehicle (for example because you're managing a parking space and nothing is allowed to be "just a vehicle" - instead you need to have motorcycles, cars, vans, whatever), then of course your design should make vehicle abstract. But there is no universal answer to that.

Also, if I store state in this class (e.g. use the fields), then would it have to be static?

No.

Answer 12

Yes it is a bad idea. Your definition of IVehicle is so general but you've still made some assumptions. The first is that the max-speed attribute may not have the same units of measure. A child of type Car may have a max speed measured in statute miles while one of Boat or Aircraft would use nautical miles. More about this sort of problem is discussed at this Codewrights Tale post on object identity. There are subtle problems with inheritance that our culture has accepted but never fully examined. If you want to understand those sublties, study how the FAA has to approach the definitions of what 'Aircraft' means so that the regulations can be defined with some semblance of sanity.