1
vote
\$\begingroup\$

I need your opinion on writing a constructor that's clean code and unit test friendly.

Option 1 :

var res1 = bigObject.GetResource1();
var res2 = bigObject.GetResource2();
var res3 = bigObject.GetResource3();
var myObject = new Foo(bigObject, res1, res2, res3);
//and the foo ctor just initializes its internal fields with given data

Option 2 :

var myObject = new Foo(bigObject);
[...]
public Foo(Bar bigObject)
{
    _bigObject = bigObject;
    _res1 = _bigObject.GetResource1();
    _res2 = _bigObject.GetResource2();
    _res3 = _bigObject.GetResource3();
}

Option 2 is cleaner (?) in the code (in option 1 I have the feeling I have to spoon-feed the Foo object because it is not smart enough to get what it needs by itself)), but requires helper methods in the tests to mock calls to Get* and pass what we want.

Of course, I can also write both ctors, and use one in the tests and the other in the "real" code, but it'd looks a bit weird and bloated to someone discovering the class, and the simple constructor wouldn't be tested.

What would you advise ?


EDIT :

Resources are not especially linked to the big object (i.e. there is no "wrong" combination). Imagine for example that the big object is a College, and resources are a list of teachers, a list of courses and a list of rooms, and we want to build, say, a timetable.

\$\endgroup\$
0

2 Answers 2

3
votes
\$\begingroup\$

Are these resources always linked to the bigObject instance? Do they have something in common with certain data or functionality in bigObject? Are they somehow related mutually? It's hard to say exactly what is the better approach without knowing all this, but there are several sanity checks you can do:

  1. Do GetResource1 to GetResource3 really all depend on the bigObject instance? Can you extract this functionality into a different class? Extract class refactoring might be an option.

  2. Does bigObject know too much? Big kinda sounds like God, as in "god object".

  3. If these three resources always come together (i.e. they all belong to the same interface), you might want to extract only them into a "parameter object". This means that you might end up with something in the middle, like:

    public Foo(Bar bigObject, IResources resources)
    {
       ...
    }
    

    Which means your bigObject will implement IResources, but you will also be able to implement them in a different way, and it will reduce the number of params.

  4. If these resources are all of the same type, and there is a large number of them, then even a simple delegate might be enough. You might even want to make the call weakly typed for greater extensibility:

    public Foo(Bar bigObject, Func<string, IRes> getResourceByName)
    {
         var res1 = getResourceByName("1");
         var res2 = getResourceByName("2");
         ...
    }
    

These suggestions popped out as I was writing, but ultimately, it's hard to give any exact guidance unless you state the actual problem you are trying to solve. Design should then follow from there.

\$\endgroup\$
4
  • 1
    \$\begingroup\$ I don't like the weak typing suggestion. Especially since each resource is most likely used in a different way, which would mean adding casts. \$\endgroup\$
    – svick
    Commented Jul 24, 2012 at 12:23
  • \$\begingroup\$ my big object is indeed close to a god object, but it's not my line to refactor this. \$\endgroup\$
    – Zonko
    Commented Jul 24, 2012 at 14:00
  • \$\begingroup\$ I updated my question to provide an example of what my objects are. \$\endgroup\$
    – Zonko
    Commented Jul 24, 2012 at 14:07
  • \$\begingroup\$ @Zonko: a good refactoring option might also be to avoid passing the actual bigObject to the Foo class entirely, and to abstract all functionality needed by Foo through one or more interfaces. This should make Foo follow Single responsibility principle, and should make it easier for testing since you don't need to mock the entire bigObject. \$\endgroup\$
    – vgru
    Commented Jul 25, 2012 at 8:50
1
vote
\$\begingroup\$

I'm not sure if this argument applies to your use case, but option 1 invites inconsistencies. Someone (even you) might pass resource values that are incompatible with each other. If you rely on any kind of relationship between bigObject and the resources, definitely use option 2.

In any case, I'd say that a clean and compact interface is more important than the complexity of the unit tests.

To make your unit testing easier, couldn't you derive from Bar and give the derived class a constructor that takes the three resources? Then you could do something like this:

var myObject = new Foo(TestBar(res1, res2, res3));
\$\endgroup\$
1
  • \$\begingroup\$ using a derived object for testing is actually equivalent to mock calls to functions. \$\endgroup\$
    – Zonko
    Commented Jul 24, 2012 at 14:02

Not the answer you're looking for? Browse other questions tagged or ask your own question.