A class should have state or dependencies, but not both

General Architecture, design, OOP
1

This principle “feels” right, but I’m working through trying to formulate it properly.

Consider a point class (in pseudo-code because lazy):

class Point
  private x, y
  construct(x, y)
  draw(Drawable surface)

(Instead of surface, it could also be a save-function with a database connection as argument (or indeed any type of persistence)).

If the surface was instead injected into the constructor, the point would look like:

class Point
  private x, y, surface
  constructor(x, y, surface)

Isn’t that weird, to have two properties related to state, and then a third property related to something completely different? Especially when it’s something effectful, like drawing or writing to disk.

On the other hand, you can imagine a command object class like this:

class InstallApp
  private db, io, logger, mailer
  constructor(db, io, logger, mailer)
  execute(app)

In this case, all dependencies are effectful classes. Makes more sense, right? And then the app class has only “pure” properties, like User or Configuration.

A rectangle depending on points is also OK, since the points are pure:

class Rectangle
  private bottomLeft, topRight
  constructor(Point bottomLeft, Point topRight)
  // draw, save, etc

Another way to phrase it is that classes should only depend on other classes in the same layer (domain layer vs “effectful” layer).

We also want to enable polymorphism, like:

shapes = [
    new Point(1, 2),
    new Rectangle(new Point(2, 3), new Point(3, 4))
]
surface = new Surface()
forall shapes as shape
    shape.draw(surface)

The major drawback is that no language can actually distinguish between a dependency and “normal” class property. Possibly they should have different semantics? Or the possibility to separate pure and effectful classes.

Thoughts?

2

Yes. That looks like a separate responsibility.

Agree.

3

But design pattern like Active Record needs an object that has both. It needs state properties that hold actual data and dependency for storing/loading those data.

4

Yeah, AR breaks this rule. Or, you could pass the db connection to the save function, like $user->save($connection);. But usually people recommend splitting entity and repository.

5

Someone recommended to replace this:

shapes = [
    new Point(1, 2),
    new Rectangle(new Point(2, 3), new Point(3, 4))
]
surface = new Surface()
forall shapes as shape
    shape.draw(surface)

With something like:

shapes = [
    new Point(1, 2),
    new Rectangle(new Point(2, 3), new Point(3, 4))
]
service = new DrawService(new Surface())
forall shapes as shape
    service.draw(shape.getDrawingData())

To not mix in too much behaviour inside the shape. This keeps encapsulation and polymorphism. Only con is that shape now needs to now about how to represent drawing data.

6

AR beaks the rule intentionally. You are trading potential problems for convenience when using it.

7

What do you think of passing the connection object to each save/load/update function? Too much administration for little testability gain?

8

Nah, not my way of doing things. Data, ideally, should be separated from what’s saving it.

9

If you want to take away dependency on DB connection from model it would be better to use DAO pattern rather than passing connection to save/load methods of model. Using the Point class from your example:

class Point 
    private x, y
    construct(x, y)

class PointDao
    private db
    construct(DbConnection db)
    load(id): Point
    save(Point point)
1 Like
10

Yes. Only problem is that you have to expose the Point internals, so PointDao can read it. Sadly, there’s no “namespace private” visibility in PHP (“internal” in other languages).

Or you can do fromSql() and toSql() inside Point if you want to import/export from a SQL-friendly format. :slight_smile:

11

You can emulate it :wink: But I don’t really recommend it since it blows people’s mind in majority of cases.

12

But we are talking about model here. It’s reasonable to assume that you will want to read/update model properties from outside of model because that’s pretty much what models are for. So, you will need to expose them anyway.

13

Ooooh! :open_mouth:

(body too short)

14

You can make them readonly by setting properties in the constructor and adding getters but no setters. No idea if that’s considered idiomatic.