Thursday 29 January 2015

Programming Books Rant

Why is it that programming books always do a lousy job of teaching you Object Orientated Programming?

The latest offender of OOP abuse!

Before I begin, just answer these questions:

  • Do you create your own classes and subclasses?
  • Do these subclasses go 3 or more levels deep?
  • You refactor common code into a parent class?
  • Do you downcast objects from a base type to a sub-type?
  • Have you ever experienced the horror that is parallel class hierarchies?

If you answered yes to any of these, its not your fault, its how programming books teach you and how most frameworks are built.


Shape, Rectangle, Square

Every book explaining OOP, uses the shape example. It is intuitive and allows the tech author to show you all the syntax and guff. It works too, all shapes implement an area() function and you can use the shape classes interchangeably, let area = myLovelyShape.area()

The example breaks down when you add properties, a circle will have a radius, a rectangle width and height, a square a length and a scalene triangle has both angles and sides. The objects may all be shapes, but now they are also unique from one another, in the sense of their API's, as they each have different properties.

You've written a graphical editor and you have a dialog box that pops up that allows you to edit the shapes properties. Wait though, the properties are different for each shape, you will need a custom dialog for each shape! So your options are:
  1. Create a custom dialog for each shape
  2. Use reflection to grab the properties
  3. Implement a getPropertiesDialog() function in the shape classes
  4. A design pattern?

Option 1

How do you know which dialog to create? I've seen this too many times:

if (myShape is typeof(Circle)) return new CircleDialog()

The problem here is you're downcasting, this if statement will have to keep growing as you add more shapes to the code. Its more code to maintain and attract bugs:

Bug #00042
Open:
  when I have a pentagon and click properties I get a message saying no such shape.

Fixed:
Added check for pentagon shape in the create dialog function

Option 2 

This is better and quite common, however reflection may grab properties you don't want the user to see or edit, so you need to pepper your shape class with annotations / attributes.

[IsEditable(true)]
public double radius {get;set;}

Also your model classes now have pseudo GUI code in them which goes against keeping the good practice of keeping data, logic and UI separate.

What if the properties are more complex such as angles in a triangle, your reflection code may check that a property is greater than 0, but it won't know that an angle in a triangle has to be less than 180°.


Option 3

This is good from an OOP perspective as the details about the custom dialog are encapsulated in the shape class, so no need for a big if statement. However you're mixing data, logic and view code which makes the code harder to test, maintain and each shape class may be be growing into 1000's of lines of code.


Option 4

No worries a design pattern or two can easily fix this, Abstract Factory and Factory method.

public static class ShapeFactory{

     public IShapeFactory createCircle() {return new CircleFactory;}
     public IShapeFactory createRectangle() {return new RectangleFactory;}
}

public CircleFactory extends IShapeFactory
{
    public IDialog createPropertiesDialog(){return new CicrleDialog();}
    public Shape createShape() { return new Circle();}
}

When a user requests the shapes dialog, you just call
shapeFactory.createPropertiesDialog()

Factory classes also allow you to easily return mock classes, so for your unit tests you could return a mock dialog that has no GUI. When you need to add a new shape, you add a new member to the ShapeFactory class

     public IShapeFactory createHexagon() {return new HexagonFactory;}

And create the HexagonFactory, Hexagon and HexagonDialog classes. Your new shape button on your toolbar can now be easily wired to call ShapeFactory.createHexagon()

Take care with design patterns tho, you can over use them (I admit I do). They make the code more verbose and harder to read sometimes.


Don't Subclass!

Subclassing is just a bad idea especially when it goes beyond 2 levels deep. The problem is that you start to add new functions and properties to the subclass. There is rule of thumb called, the Liskov substition principle which basically states that subclasses should have the same public methods and properties, this way there is no need to ever downcast or worry about special cases.

Instead of subclassing you should use the 'has a' relationship instead of 'is a'. Which means instead of extending a class, you should instead use an instance of the class as a property.

//square 'is a' rectangle
class Square : Rectangle {
}

//square 'has a' rectangle
class Square
{
    private Rectangle {get;set;}
}

The 'has a' Square re-uses code in the rectangle class without the restriction of being a rectangle type, leaving you more room to maneuver in the future. You will also save a yourself a lot of hassle, as if you make a change to a super class, inevitably you have to propagate the changes to all its child classes. 'Is a' makes your classes more tightly coupled than 'Has a', tighter the coupling the more easier it is to break your code when you make changes.

If you require a class to conform to an API, use an interface:

class Square implements IShape{
    override double getArea()
}

I know it might be tempting to subclass, especially if it you can re-use a few lines of code in the base class, but don't, there's nothing worse when trawling thru code and you go up a class hierarchy with parent classes that just have a few piddling lines of code in them serving no other purpose but to reuse a few lines of code. Use standalone utility classes if you need to reuse some common code.

When to Subclass

Martin Fowler, an OOP fanatic,  recommends to use subclasses instead of enums.

enum Color {Red, Green, Blue}

He argues that enums are sub types of a common super type, and instead of using ints to differentiate between enum members we should use the OOP tools at our disposal. 

Procedural code would look like: 

String getFloweryDescription() {
switch (color)
{
   case Red:
       print("Rose Red")
....

OOP code
class Red  extends Color
{
    override String getFloweryDescription(){return "Rose Red";}
}

The advantage here with OOP is that its easier to new add types, using enums tho we would have to remember to update any switch statements.

Personally I still prefer enums and switch statements, easier to read and quicker to type than a bunch of wee classes. 


Frameworks Use Subclassing

Hang on a minute you say, Android SDK has frameworks that use subclassing quite extensively. I mean you subclass Activity and override lots of various methods to add your own behavior. Why is subclassing OK here?

Quick answer its not OK, I think that most frameworks after a few years become to unwieldy and complex. 

Graphical API's naturally form sets and subsets which lead to API's that use deep layers of subclassing. The eggheads who first designed the Android graphical library, did a nice clean job. Overtime as new features got added, it became necessary to put some awful bodges into the code. The problem with deep hierarchies is that they are difficult to change. As a developer using the Android SDK, it seems big, difficult to adapt and small things seem to require complex work a rounds. 

As a developer it is rare to come across any need to create your own class hierarchies, they just don't exist in most of the day to day coding. The Shape example is so ubiquitous because its hard to come up with other good examples of hierarchies. Saying this, I've come across many unnecessary examples of useless class hierarchies. I know whats going through the devs mind, OOP is cool and I want to use it as much as possible. Myself I'm no different, currently I'm mad on functional programming, before that design patterns. You only learn by your mistakes I guess. 


To summarize, Keep It Simple!
PDB





No comments:

Post a Comment