Is having empty children classes, just to specify, a bad practice?
Lets suppose I’ve a generic Product class. There are electrical, electronic and mechanical products. I need to represent all them and must be able to differ each one in some operations.
I’ve two options:
-
add a member named “type” in Product class to specify whether it’s a electrical, electronic or mechanical product.
-
create children classes of Product, one for each kind of product: ElectricalProduct, ElectronicProduct and MechanicalProduct. As these classes don’t differ from each other in methods or members – already defined in parent class – they would be all empty.
I consider second option a better approach, but is it the right way? I’ve never seen such thing on any application or library.
What do you thing? Any suggestion?
Edit:
Here is an example of doing it using second option: https://gist.github.com/tsouzar/c2ffebbcdad30920bf06
I can’t see a better way of doing it, but I don’t know if it is a good practice to have that empty classes.
11
Now that you’ve explained more fully, it seems that there really is no subclass-specific logic. In that case, inheritance is not the right choice. If there really are only minor ways in which other parts of the code treat products, putting code into product classes would be wrong – a violation of separation of concerns which would encumber the product classes with knowledge of other parts of your system. The sorting example makes that clear. Products should not know how they are sorted or stored or presented; if the code which sorts or presents objects is changed, the product class should not need to be rewritten.
I apologise for doubting you on that. We see so many people here presenting questions which are based on poor OO knowledge and I assumed this was the case when I should have asked for more info first.
In fact…
The method overloading choice, as shown in your example code, is actually going against principles like the separation of concerns and DRY. What is the common thing about the add_product method? It adds a product to the appropriate collection. But in the example code, that single, fundamentally identical task is duplicated in three different places. This is really scarcely different to a chain of if..then..else logic.
- The logic is duplicated and any changes to the common behaviour will have to be duplicated in each place. Mistakes are likely and will go undetected.
- The product-adding code should not need to know what types of products are available. All it needs to know is that there are a variety of products, each of which should be added to an appropriate collection. Making it aware means you have to change this section of the code every time you add a new product.
A better way to do this, if you really do need a separate collection for each type of product, would be to have a map of product collections. The key would be the product type. Then you could have one single product_add method, which consults that map and adds the product to the right place. Even if you discovered that there is some need for different product subclasses, this method doesn’t need to know. The object class itself could be the key to the map, for example.
Since more than one method is likely to want to update or read from those collections, it is probably better to have a Products object which stores the objects. Whether it contains a map of N different collections or one collection which it knows how to filter/search by type is an implementation detail which need not concern other parts of your code.
In Summary
Since there really seems to be no product-specific code, extra empty classes are not only useless, they complicated your code and make it more fragile. A ‘type’ member is better.
- Most of your code doesn’t even need to know that there is such a member.
- In each domain (the clearly separate sections of your application), identify those few bits which genuinely need to differentiate between product types.
- Isolate them in special helper classes or functions which will return the appropriate product (or do the appropriate thing which matches the type)
- Maps are very useful (they can contain not only collections but other helper objects/functions to hand back to some other code to do the appropriate thing with this object) but should be hidden from anything but the privileged code which really needs to be product-type aware.
If you do that, not only will your code be cleaner and more elegant (free from ugly if..then..else chains, for one thing), if you ever do find a real need for product subclasses, almost none of your existing code will have to change. And even they need only change their internal implementation, because their code will still be no concern of the product class or any of its subclasses.
There is still plenty of fun to be had with adding new classes in the respective domains to encapsulate any “I know what kind of products there are”. One challenge is differentiating between the bits of those code which are domain specific (e.g. sorting in a collection which is only relevant in one domain) from those which are actually general. The list of all valid product types, for example, should be an enum which can be fetched by any product-aware domain-specific code. Any map should use that enum as the type for its key. Any function which takes a product “type” as a parameter should take a parameter only of that enum type and so on. Now, where in your code are you going to define that enum?
6
I think the reason you are having trouble with which path to take is because there is a fundamental design flaw here. You have anemic classes here, which is itself an OO anti-pattern. The external logic that is switching on the type needs to be brought into the Product
family of classes.
The code smell for this is that, no matter which path you go, you will either need to check a type member or instanceof
in other places in your code. The other smell is that you will constantly be querying a Product
object for data and making decisions based on it (see tell-don’t-ask). This means that it will be difficult to modify or add behavior because you will be needing to make changes in many, disparate places rather than in an appropriate class to hold that behavior for each type.
Edit:
The addition of the code details shows there is a little more nuance to your question. Let’s start by looking at what we are talking about by “type” of product.
Traditionally, anytime we hear the word “type”, we jump to the object concept of Type (which I will capitalize to differentiate). This Type is associated with polymorphic behavior, inheritance, encapsulation, and all the other object-oriented concepts. Here, though, we aren’t necessarily describing a Type. We are talking about what looks to be a data attribute called a type. The behavior does not really change based on the type, only the value.
As an example to compare against, we can look at integers. We have int
s 1
and 2
. The behaviors between them are exactly the same; only the values differ. This means that we shouldn’t have an abstract type for int
that is implemented as a int1
concrete type and an int2
concrete type, etc. We need one concrete type for all int
s, and a simple way to interact between them (like comparison for sorting). Your idea of a product type is the same.
If your products have or will have distinct, polymorphic behaviors, inheritance is a valid solution. Here, though, your type is only a data attribute, like the price and the brand, with no difference in behavior. Therefore, you should have a type attribute like you have a price attribute and a brand attribute.
Ideally, you will want to make your code more open-closed, so there should be a way to do this as elegantly as possible. We can create an enum containing the different types of products that exist, ordered in the order they need to be output (using a static std::set or similar on product
might be easier to work with, if you’d rather). The type attribute of the product
will point to an enum value. In your product_manager
class, instead of three named vectors, you should have a map/dictionary-type structure to map from type enum to the respective vector, generated from the elements in the enum. This way, that structure will automatically add vectors for each type at instantiation. When you add a product, select the vector to add it to based on the type of the product. Then you can iterate over the map, then over each vector to print out the products, grouped by type.
Pseudocode:
enum product_type { electrical, electronic, mechanical }
class product {
//...
get_type() {
return this.type;
}
//...
}
class product_manager {
map<product_type, vector<product>> products;
product_manager() {
foreach(p_type in product_type) {
products.insert(p_type, new vector<product>);
}
}
//...
add_product(product p) {
products.at(p.get_type()).append(p);
}
//...
print_products() {
foreach(p_type in product_type) {
foreach(product in products.at(p_type)) {
print_product(product);
}
}
}
//...
}
If you never want to use the type anywhere else, you can make it private, and make the product_manager
a friend to product
.
I also want to make a quick point about the example code you posted.
<code>//in real application we don't know instantiation proccess, so don't know what type of product comes to us.test::electrical_product *ep = new test::electrical_product;test::electronic_product *etp = new test::electronic_product;test::mechanical_product *mp = new test::mechanical_product;// set products' members valuespm.add_product(ep);pm.add_product(etp);pm.add_product(mp);pm.print_electrical_products();pm.print_electronic_products();pm.print_mechanical_products();</code><code>//in real application we don't know instantiation proccess, so don't know what type of product comes to us. test::electrical_product *ep = new test::electrical_product; test::electronic_product *etp = new test::electronic_product; test::mechanical_product *mp = new test::mechanical_product; // set products' members values pm.add_product(ep); pm.add_product(etp); pm.add_product(mp); pm.print_electrical_products(); pm.print_electronic_products(); pm.print_mechanical_products(); </code>//in real application we don't know instantiation proccess, so don't know what type of product comes to us. test::electrical_product *ep = new test::electrical_product; test::electronic_product *etp = new test::electronic_product; test::mechanical_product *mp = new test::mechanical_product; // set products' members values pm.add_product(ep); pm.add_product(etp); pm.add_product(mp); pm.print_electrical_products(); pm.print_electronic_products(); pm.print_mechanical_products();
You use method overloading with different parameter types to determine which implementation to call. The trouble with this is that, in your real application, all the products would be of type abstract_product *
. The compiler routes the method calls based on the compile-time type of the object, meaning any products being added would be sent to a method product_manager.add_product(abstract_product *)
instead of the overloaded methods, meaning that it would not do what you intend.
5
My opinion is that adding a type member is a better solution. It’s simpler to define, allows you to treat the type as a first-class value, lets you code Product as a plain old data structure (which it is), works without RTTI, and you can use a switch
instead of multiple if
/else
to efficiently branch based on the type. Using inheritance your code can break if someone introduces a new subclass, since you’re only expecting a finite number of types of products.
4
I think it’s best to have one Product
class with a type field, for a few reasons:
- If you ever need to serialize/deserialize in a way that the language doesn’t natively support, there’s only one constructor; you don’t need to switch on the type and use one of several constructors.
- There’s a slight benefit to readability – if readers see
ElectricalProduct
they might wonder if it contains special logic, and have to open it to see that it doesn’t. - You can make
Product
final (Java) or sealed (C#), which is good for efficiency – certain method invocations won’t need to go through a vtable.
That said, if you think you might later add some logic specific to certain product types, then separate classes might be better so that you can do so without a major refactor.
8
If indeed you “must be able to differ each one in some operations”
as you say, introducing three such intermediate classes is a perfectly
sensible idea.
Have a look at the idea of Abstract Class (e.g. in Java).
An abstract class cannot be instantiated itself (most OO languages
have a respective construct that will technically
forbid instantiation) but it provides variables and methods
to its children — and each child class still needs to add some bit
to make a complete, instantiable class.
In your case, ElectricalProduct, ElectronicProduct and MechanicalProduct
would be abstract classes.
4