Don’t Be So Classless

Over the last 3 or 4 years, I’ve started to get frustrated with something I see a lot in code. It’s a two-headed monster. The first head is lack of classes, and the second is not properly using the classes that do exist.

No Class

First things first, I find despite the fact that my entire professional career has involved C++ (7 years), C# (5 years) and Visual Basic.Net (1 year), all of which can be called “Object Oriented”, most of the code I interact with doesn’t have enough classes. I think there is a unspoken idea that more classes is bad design. It’s as if we decided a while back that having fewer large classes that do several different things is better than having more, smaller classes that each do fewer tasks. 

Because of this mindset, it’s as if we’re afraid to create classes. Instead, we pass around a lot of parameters. The gist below isn’t an exact code snippet that I’ve worked on, but it’s similar enough to call it a pseudo-real-world example:

https://gist.github.com/4570406

I’ve written and seen controllers like this seemingly a thousand times. When looking at code like this I now ask myself “Who’s responsibility is it to validate the data?” In the sample above, it is the controller’s responsibility. But a controller’s responsibility is supposed to be more of a dispatcher or organizer. It takes incoming requests, farms them out and returns the results. It’s the infrastructure of the application.  So the teams I’ve worked on have started adding logic classes. The new logic classes make the above snippet look like this:

https://gist.github.com/4570440

At least with this snippet, the controller is acting more like a controller. It’s receiving a request and returning a view. Everything else is done outside.  But we have to ask the same question as above, “Who’s responsibility is it to validate the data?” In this case, it’s the UserLogic class. Maybe that’s ok. Maybe we’ve pushed it down far enough, but I don’t think we have.

Right now all we’re doing is creating a new user, what happens when we need to update a user?  We’d probably have something like this:

https://gist.github.com/4570490

Now we have a problem, we have the same boilerplate validation logic in two methods. What if we have some other operation on the User controller that needs to validate the same parameters?  If we weren’t class averse, we could easily clean this up to look like:

https://gist.github.com/4570557

Now we have a user class that is responsible for it’s own validation. After all, who better to know if an object is valid than that object. This has the benefit that everyone that needs to validate a user object can reuse the validation, and reduce the risk of not validating part of the object.  We also have an update method. This allows the user object to know any special logic that might go into updating the object. In our case, it’s just a straight update, no special logic.

Not Using What You Have

The final snippet above shows us using our User object to do validation. However, the second part of the problem is not using the classes you have.  For example, we might see something like this:

https://gist.github.com/4570586

Here our User object is really nothing more than a DTO. Instead of asking the User object if it’s valid, we’re asking it for all the information and then deciding if it’s valid. This would be akin to you pulling your car into your garage and when you went to turn it off the car asked you “Am I in park?” and if you said “yes” it would let you turn it off. If you said no, it would hold on to your key. It doesn’t make sense with cars, and it doesn’t make sense with objects.

How Did We Get Here?

I’m not entirely sure how we got here. I think some of it is just human nature. As we’re developing, we’re thinking we need to validate the object. It’s easier for us to write the code in the controller or the logic class instead of creating a class. After all, thats where we’re at when we start thinking about validation.

However, I think there is something to be said about some of the language features as well. For example, for about the past 5 years, C# has given us Object Initializers. Before this, the only way to set fields of an object was to create a constructor. We’d often wind up with code like this at the top of our class:

https://gist.github.com/4570631

For every combination of parameters that we might need for this class we needed a unique constructor.  We also did things like copy constructors, where you’d pass in another User object and copy it’s fields to a new User. This works well if you know beforehand what fields your users will want to instantiate with.  It also works reasonably well for classes with a few fields. If your class has 10 or 15 different fields, the constructor permutations become cumbersome.

However, the constructor permutations might be a good time to ask yourself if you need to be creating a class with that many fields. Is it really true that the majority of your methods on that object use a majority of those fields? Or are there actually classes that exist within your class?

Now, we have object initializers, so we can create objects just like this:

https://gist.github.com/4570666

I have found when I create objects in this manner, I think of them less like objects and more as data containers, or really, a property bag. Something to just hold a bunch of somewhat related data.

Another language feature that helps us wind up with the bad code above is the way C# handles properties. When I first started writing code professionally, I could either make public fields (which was a good way to get my code thrown out during a code review) or write getters and setters for my properties. The second method usually meant that you didn’t expose all your properties or fields because you wouldn’t want to write 2 methods for each property.

However, now, I use properties in C# almost exclusively. I do typically have a couple private fields on the class, but most of the fields are actually properties. It’s so easy to implement, and it makes it easy to grab whatever data from an object that you want.

It’s not all negative, however. These features were required for C# to implement some of their LINQ functionality. They made things like Entity Framework, LinqToSql and other features of C# possible. The features themselves aren’t bad, but how we use them can be.

tl;dr

It’s not bad to have classes, in fact most of the time it’s good. Not only does having classes provide you with reusability, it also lends to cleaner code. It should also help make more clear who has what responsibility. So let’s get out there and add some class to our code.

 

 


3 thoughts on “Don’t Be So Classless”

  • 1
    Erin on January 19, 2013 Reply

    I think that this begs the larger discussion of how powerful do you want your objects to be? What if part of the validation requires a database hit? Do you empower your object classes to do that? If a user object validates itself, does it also save itself? While I agree with your argument, It’s a major decision in any application, and the approaches shouldn’t be mixed, IMO.

  • 2
    Nate Taylor on January 19, 2013 Reply

    Yeah that's something I've debated with myself. Obviously ruby does it with active record. I'm still not convinced either way. Just realized a year ago that my code isn't always maintainable because I do validation etc in lots of places

  • 3
    Shaun Bedingfield on February 27, 2013 Reply

    I like your comments and your thoughts. I like to consider factoring a system an art. You make a lot of decisions with no clear right or wrong answer and at the end of the day, one system seems obviously more elegant than another. If you try to build two systems exactly the same, you will likely get working code but it will likely be less than ideal.

    It is much easier to know what a improperly factored system is than to build one and technical questions often have no right answer. Sometimes, the database is the best place to validate, sometimes you need to validate in the UI too, and sometimes code works better. It is about technology, business needs, team members, and a slew of other problems. The more you have to balance, the more obviously impossible it is to always pick the one right solution or that there is such a beast.

    I think one thing that is clear is that large classes in general do not work and in general one is better trying to break a design into the appropriate concepts and assign responsibilities. Of course, we could always go into overly long database code where you pray for CTEs and inline functions and views and cringe when you see regular functions reducing code to cursors or poorly structured unreadable long SQL queries. The problem is kind of universal and it is not an easy problem but you are a better developer for thinking about it.

Leave a Reply

Your email address will not be published. Required fields are marked *