Active vs. Passive code bases
I was review code at a customer site, and he had a lot of classes that looked something like this:
1: public class ValidationData2: {
3: public string Type {get;set;}4: public string Value {get;set;}5: }
In the database, he would have the data like this:
This is obviously a very simple example, but it gets the job done, I think.
In his code base, the customer had several instance of this example, for validation of certain parts of the system, for handling business rules, for checking how to handle various events, and I think you get the picture.
I seriously dislike such codebases. You take an innocent piece of code and make it so passive it… well, you can see:
Here is why this is bad. The code is passive, it is just a data holder. And that means that in order to process it you are going to have some other code that handles that for you. That likely means a switch statement of the equivalent. And it also means that making any sort of change now have to happen on multiple locations. Puke.
For fun, using this anti pattern all over your codebase result in you have to do this over and over again, for any new interesting thing that you are doing .It is a lot of work, and a lot of places that you have to change.
But you can be a hero and set the code free:
You do that by making a very simple change. Instead of having passive data containers that other pieces of the code need to react to, make them active.
1: public class AvoidCurseWordsValidator : IValidator
2: {
3: public string[] CurseWords {get;set;}
4: public void Validate(...) { }
5: }
6:
7: public class MaxLenValidator : IValidator
8: {
9: public int MaxLen {get; set;}
10: public void Validate(...) { }
11: }
12:
13: public class InvalidCharsValidator : IValidator
14: {
15: public char[] InvalidChards {get;set;}
16: public void Validate(...) { }
17: }
Now, if we want to modify / add something, we can do this in only one spot. Hurray for Single Responsibility and Open Closed principles.
SO… don’t let your codebase be dominated by switch statements, parallel hierarchies and other nasties. Make it go active, and you’ll like the results.
Comments
Data driven development is still very popular, even though object orientation was invented decades ago:)
It looks like they are using EAV model, which is bad when used in wrong place, but in some cases it does its job. Take for example magento e-shop - it uses eav a lot (was using even more before) and it is popular, because it is flexible and does it job, it scarifies performance over flexibility.
EAV may be interesting (event though I strongly hate it) in relational database, but it's a huge mistake in a schema-less database.
@Remi, could you explain why it is a huge mistake in a schema less database? Because out of context, but Ayende's first sentence below kind of says opposite, but then later it kind of correlates with what you are saying, so I think I'm missing the binding piece here to have "aha" moment.
From: Oren Eini Date: 29 February 2012 17:04 Subject: Re: RavenDB and EAV To: Giedrius
Oh, no. RavenDB is awesome for EAV. The schema less nature fits RavenDB very nicely. You can have different properties for different docs, different properties for different docs of the same type, etc.
You can search and work with that effectively.
It is a really sweet spot for that.
On Wed, Feb 29, 2012 at 5:02 PM, Giedrius Banaitis wrote: Hi, I'm wondering if EAV (http://en.wikipedia.org/wiki/Entity%E2%80%93attribute%E2%80%93value_model) is as bad in RavenDB as in relational database based solutions, or RavenDB is well suited for such problem approach (if you need more concrete domain to answer - products and their attributes)?
Giedrius
In a document database you would have the attributes and values embedded in the document itself, and not in a seperate document / table as done in a relational database.
I think a lot of this design pattern comes from a requirement (either real or just perceived) to be able to change or fix the functionality of an application live without having to re-compile code and redeploy. Sometimes it's necessary, and sometimes it's not, but there's no silver bullet - just trade-offs to different approaches. I think this is one of the main reasons stored procedures (shudder) continue to live on.
@Matt,
If I had a nickel every time I heard 'you should be able to change ? just put it in ?.config!' and the justification is always 'the business shouldn't have to go to a developer everytime that blah.'
Of course said systems eventually become un-maintainable.
Dat teddy bear.. haha!
But yea, I agree with this post. In my experience the 'passive' style just causes unnecessary confusion as you waste time and mental energy searching for which data object is related to which consuming class, yea, loosely coupled but for what reason?
Dan, THANKS YOU, So many comments and you are the first to comment on that.
@Giedrius, you mentioned Magento as a example why EAV models in relational databases are good. This is completely wrong. I work with Magento on a day to day basis and every single developer I've met has complained about Magento being so slow. The top reason for that is EAV, so Magento 2.0 that is announced to go stable later this year will break with EAV at a lot of different parts. So when the most popular eCommerce system in the world decides to go away from EAV because it's slowless, I don't think this is a good point for that pattern.
eCommerce systems could be so much more powerful if they'd use a document database like RavenDB but unfortunately that news hasn't yet arrived at the PHP/MySQL crowd.
Does Value Object (VO) pattern do a better job than this data-driven/ validations style?
In some companies having to redeploy could be a full days work, while doing a database update might be an hours work (paperwork, ...). This, among others, might be the reason to use
Switch statements (or excessive use of them anyway) are usually bad in OO design but it's not the Type/Value table in the database that is at fault here! You can define AvoidCurseWordsValidator but simply get the list of curse words out of the database.
Because in the end, you will need to set the AvoidCurseWordsValidator.CurseWords property. You still need to inject those form somewhere. A database sounds like a legit option to me here...
Laoujin, I think you miss an important point. What I want to do is to store the entire validator in the db. Not just the data for that.
When people combine methods and data into a class in a way such that you are recommending, I wonder if they truly value the single responsability principle. In my mind, storing both schema and behavior in the same class qualifies as a violation of the SRP. Do you disagree with me that this is a 'violation', or do you just not think the SRP is important?
Jason, I would strongly disagree here, see my next post for details
TIL: Ayende is apparently secretly working at my place of employ. (o_O)
Seriously, this is an example of the code base I recently got thrust into. It truly is a pain to deal with, and trying to clean it up is even more so.
Comment preview