Pages

Tuesday, February 2, 2016

Evil Traits or Poor Programming Skills

So I came across this nice little article about how evil Traits are, and I thought to my self, here is yet another developer who thinks we should all work in Assembly. Are traits evil? Of cause not. Can they be used in a wrong way? Sure they can. So can functions, classes, interfaces and so on. But you cannot blame traits for peoples bad choices. Now there is a lot of reference in this article to people with C++ background saying how wrong traits are, as these references should some how prove a point? I am positive that I have seen the same points being made against OOP by people with C background.

Let's take an example of trait usage. Let's say that we want to build an OOP database abstraction layer so that we can easily adopt code for other databases. Here is what we need for basic features.
  • Select Query class
  • Delete Query class
  • Insert Query class
  • Update Query class
Also we want some shared tools such as 'compile', so let's make an abstract class that the above mentioned can extend from, let's call this 'QueryBuilder'.

abstract class QueryBuilder {}

class SelectQuery extends QueryBuilder {}
class UpdateQuery extends QueryBuilder {}
class InsertQuery extends QueryBuilder {}
class DeleteQuery extends QueryBuilder {}

What does all 4 classes needs for starters, well the options of choosing one or more tables. No mater the query type, you will need to define at least one table. So this one is best put in 'QuryBuilder'. In the above mentioned article he wrote that one problem with traits was that you could change the access level of methods when adding them to a class. Let's get an example of why this is useful. Most databases, if not all, does not really need multiple tables for an insert operation. If we add table features to the 'QueryBuilder', then this would be possible. Let's go another way. Let's create a trait instead and see where this takes us. We will call this `QueryBuilder_Table` where we use underscore as naming convention since it's a cut of part of 'QueryBuilder'.

trait QueryBuilder_Table {
    protected function table(string $name, string $alias=null) {
        // ...
    }
}

Now that we have the table feature in a trait we can add it in different ways to each class. All 4 classes excepts a table name and an alias in their constructor. But since we have made the 'table' method protected in the trait, we can extend the features in some of the classes by simply changing the access level.

class SelectQuery extends QueryBuilder {
    use QueryBuilder_Table { table as public; }

    public function __construct(string $table, string $alias) {
        $this->table($table, $alias);
    }
}

class UpdateQuery extends QueryBuilder {
    use QueryBuilder_Table { table as public; }

    public function __construct(string $table, string $alias) {
        $this->table($table, $alias);
    }
}

class DeleteQuery extends QueryBuilder {
    use QueryBuilder_Table { table as public; }

    public function __construct(string $table, string $alias) {
        $this->table($table, $alias);
    }
}

class InsertQuery extends QueryBuilder {
    use QueryBuilder_Table;

    public function __construct(string $table, string $alias) {
        $this->table($table, $alias);
    }
}

Now your able to define multiple tables in classes such as 'Select', but not in 'Insert' while using the same trait. Could this be done with normal inheritance, sure it could, but we are not done yet. Also I might add that extending a class with normal inheritance also allows you to change the access level. So if this is something that makes traits evil, then maybe someone should read a little more about the options with class inheritance.

Fields is another great thing to add to a query, but we have two different scenarios. 'Select' for example defines fields/columns that should be returned while 'Insert' and 'Update' defines fields to insert/update with a value. So for this we make two traits 'QueryBuilder_Field_Selectable' and 'QueryBuilder_Field_Updatable'.

trait QueryBuilder_Field_Selectable {
    public function field(string $name, string $alias=null) {
        // ...
    }

    public function fields(string ...$name) {
        // ...
    }
}

trait QueryBuilder_Field_Updatable {
    public function field(string $name, $value) {
        // ...
    }
}

Of cause we exclude 'Delete' as it does not need either of these options. Let's move on to conditions, something needed for 'Select', 'Update' and 'Delete'. Again we create a trait 'QueryBuilder_Conditional'.

trait QueryBuilder_Conditional {
    public function condition(string $field, $value, string $operator="=") {
        // ...
    }
}

We could go on from here and add more, like join features etc. But let's move along to something else. We have 3 classes that share one thing, 'Delete', 'Update' and 'Insert' does not need to return a result set. Instead they would be better of returning a number of affected rows. This is a feature that is actually worth checking for, so it would not be well placed in a trait. Instead we create another abstract class named `QueryExecuter` that extends from `QueryBuilder` with a method called `execute()`. At the same time we add a similar `enquire()` method to 'Select' that returns a result set. This leaves us with 5 actual data types 'QueryBuilder', 'QueryExecuter', 'Delete', 'Insert', 'Update' and 'Select'.

abstract class QueryExecuter extends QueryBuilder {
    public function execute(): int {
        // ...
    }
}

The great thing about using traits in this way, is that we get a proper separation of the different functionality. Sure 'Update' and 'Select' could have a shared class with conditional options like a `QueryConditional` for example. But that would actually be wrong and provide a very poor design. You would be better of implementing the features separately in each class. Even though they share some similar methods, the functionality they use those methods to provide is completely different. One updates a set of rows/columns while the other fetches and returns a set of rows. So when would it ever make sense to check for 'QueryConditional'. Using traits they can share these tools without having any relation with one another. Sharing 'QueryExecuter' does make sense, because it provides the exact same functionality. It executes the query and return the number of rows affected by it.

In the above mentioned article, one of the claims for traits being so evil was that classes does not inherit the data type of a trait. Well thats sort of the point, and when used correctly it's actually a good thing. You are not meant to check for options provided by traits. You are meant to check for options provided by a specific class. How that class obtains those options is beside the point.

Let's take a last look at the first example. We could have added the 'table()' feature to 'QueryBuilder' as with a protected access level and then overwrite 'table' as public and call `parent`. But there are two things that traits does better in this scenario. Firstly you don't need to overwrite it and make an additional call to 'parent' in order to change the access level. Secondly what if you want to use 'QueryBuilder' for something else that should not have 'table' options. Maybe create a 'Condition' class that can be used to add multi-level conditions.

In this case you could extend your 'QueryBuilder_Conditional' to except 'QueryBuilder' instances, and you could use your 'QueryBuilder_Conditional' trait within your 'Condition' class to gain the conditional features. Remember that 'QueryBuilder' is used to provide the 'compile()' feature, which is not exclusive to 'QueryExecuter' or the other classes that we have made so far.

trait QueryBuilder_Conditional {
    public function condQuery(QueryBuilder query) {
        // ...
    }

    ...
}

class ConditionQuery extends QueryBuilder {
    use QueryBuilder_Conditional
}

In conclusion Traits are in no way evil. They are a brilliant addition to PHP and will save people a lot of time. Sure they can be used wrong, but as mentioned above so can everything else.