views:

223

answers:

6

Hi,

I'm currently doing some refactoring (+ adding new features) to some of our framework classes. The situation is that we have a single (god-like) class which does a bunch of logic we'd like to split up. The class represents something like a validation rule for fiscal codes. So it does validation of the names of the person, birthdate etc..

What I am going to do is to split it up in single rules, basically a rule which validates the person's firstname against the fiscal code, another one for the birthdate and so on. For the programmer at the end it looks nearly the same. Instead of invoking the huge constructor of the FiscalCode rule, he'll do something like FiscalCode.GetRules(...) and pass the parameters here. The GetRules(...) will then internally construct the single rules and pass them back as an array. That's perfectly fine and correct for us.

So much for your background. Now my question is the following. The FiscalCode class (which is our current mighty god-class) has a lot of utility methods which will be needed by more of the single "rule classes" I'm going to create. What I know is that I will somehow still need the FiscalCode class, for doing the GetRules(...) thing (this is to remain constant somehow for the programmers, not that they have to do a completely new thing).

I have two options which come to my mind:

  1. Create my new rule classes and access the public static utility methods of the FiscalCode class
  2. Create my new rule classes as inner nested classes of the FiscalCode class s.t. I have already access the utility methods (and therefore no need for exposing my utility methods)

I have already a favorite, but I'd like to hear the opinion of some of you first.

Thx

+2  A: 

As your methods became 'utility methods' you need to make them static and public, but probably you need to rename your FiscalCode to FiscalCodeUtil. So it will be obvious what kind of methods it contains.

Paul Podlipensky
well, the thing is that these are utility methods, but they will just be used within the group of FiscalCode rules. So exposing them publicly is not really correct, although it wouldn't be a problem. With nested classes I could directly access the private static methods of the FiscalCode rule.
Juri
A: 

What dependencies do these utility methods have on the FiscalCode class or the rule classes? Is there state kept by them?

If there aren't any dependencies I'd suggest moving those utility methods to a seperate class, and have the FiscalCode class or rule class call into those methods as appropriate.

For the options you give, the only difference between 1) and 2) is whether the rule classes are visible to classes that don't use them. I don't think thats really an important objective. I used to worry about that all the time when I did c++... it was a waste of time.

Frank Schwieterman
A: 

IMO you should go for the first option because that way, you can expose the newly created classes to outside world, and can write code that is reusable elsewhere as well. If you go with the second option, you are creating very specialized classes. Your outside code may not even know of its existence, but that might be good for encasulation. Still, at some point you may decide to use the specialized rules outside the scope of your larger class, and for that scenario, you are better served with the first option. What is your pick though?

A: 

If the class will not be used outside the FiscalCode class, then make it nested. The important thing is to pull the responsibility of this new class out of FiscalCode; where it resides then becomes a mere question of choice. When the new class gets more dependents, you could make it an outer class.

jeyoung
A: 

I would go with it like this (I'm not that good at OOP so take it with a grain of salt):

Rule classes (nested in FiscalCode) implement an IRule interface exposing rule methods (like Validate(), with whatever return type floats your boat). FiscalCode has an AddRule() method which manages an internal collection of rules and returns a reference to self in order to permit method chaining:

FiscalCode fc = new FiscalCode();
fc.AddRule(new RuleClass1(<params specific to RuleClass1>)
  .AddRule(new RuleClass2(<params specific to RuleClass2>)
  ...

Also, FiscalCode has a Validate() method which iterates through each rule's Validate() and manages errors.

IMO this is quite handy to use and still permits to nested rule classes access FiscalCode's utility methods.

Sorin Comanescu
:) that's already implemented. We have already a full-featured hierarchy with rules etc. My purpose here was just on how to put the FiscalCode rule into relation with the specialized more fine-grained rules. Thx anyway.
Juri
+1  A: 

I would also suggest a review of the Specification Pattern, which gives some direction on how to approach this type of problem. This post also gives some examples in C#.

The suggested Specification Pattern would steer you towards your option #1.

Chris Melinn
Thanks for the suggestion about the pattern. I didn't know that pattern yet. I'll take a look at it.
Juri
If you get the chance to try it, let us know how it goes. Best of luck!
Chris Melinn