tags:

views:

101

answers:

5

I need to design a method that can potentially take as a parameter an object, if it doesn't then the method has to create a new object by itself.

Is this a good way to do it?

public void Method1(int companyId, int userId, int clientId)
{
     Method1(null, companyId, userId, clientId);
}
public void Method1(SpecialObject o, int companyId)
{
      if(o == null)
          o = new SpecialObject(userId, clientId);


}
+1  A: 

I wouldn't do that. It sounds like that is outside of the scope/responsibility of Method1. Why cant you construct SpecialObject before calling Method1?

Daniel A. White
To add to what Daniel said, the SpecialObject constructor requires userId and clientId, which Method1 has but which Method2 does not have.
ChrisW
A: 

Looks like a perfectly good design pattern to me.

(I don't know if your initialisation of "o" is valid though, but that's beside the point)

I've used something similar when writing unit tests.

In the normal (non-test) case the class under test constructs it's own member objects. however, when it's under test I can pass in a mock or a stub to use, if it's appropriate.

For example, here's some C++(I know the question isn't about C++, but the technique is transferable) code to illustrate the technique I've used.

class Foo {
public:
    Foo() : m_bar(new Bar()) {}//normal (non-test constructor)
    Foo(Bar* bar) : m_bar(bar) {} //constructor used when unit testing

private:
    Bar * m_bar;
};

The difference here, is I use this with constructors, not general methods

Glen
A: 

This is not a good design. If Method1 is a create operation, it should return the object being created. If it is a regular update operation, it should require that the object was created in advance.

Most likely, the body of Method1 should really be a method of SpecialObject (which would then probably only receive companyId).

Martin v. Löwis
+5  A: 

Expanding on Daniel's answer, I've often used this pattern:

public void Method1(int companyId, int userId, int clientId)
{
     Method1(new SpecialObject(userId, clientId), companyId);
}

public void Method1(SpecialObject o, int companyId)
{
    // if needed
    if (o == null)
    {
        throw new ArgumentNullException("o")
    }
    ...
}
Michael Petrotta
+1  A: 

Your code snippet is not clear. In your first override, you call a Method1 override that you haven't shown. In your second override, you pass userId and clientId to SpecialObject's constructor, but do not show where userId and clientId are defined.

Regardless, some general approaches include

  • ordering the parameters such that the ones that appear in more overrides are positioned first,
  • expecting reference variables to be non-null (check and possibly throw ArgumentNullException), and
  • for reference variables that can be null, create an override that doesn't include that variable.
gWiz
+1, especially for the first point. Since companyId appears in both, it should really be first. That way, the rest of the parameters are either a parameter list or an object instance/value containing all of them.
Zooba