HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Different Constructors, Same Implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
constructorsdifferentsameimplementation

Problem

I have this class containing two constructors with different signatures but they do the same thing:

public Person(Dictionary dictionary, string someString)
    : base(dictionary, someString)
{
    base.GetProperty("FirstName");
    base.GetProperty("LastName");
}

public Person(Dictionary dictionary, string[] someStringArray)
    : base(dictionary, someStringArray)
{
    base.GetProperty("FirstName");
    base.GetProperty("LastName");
}


The type of the second parameter in each of these constructors determines the behavior of the 'GetProperty' method.

I've read about casting and the bad behaviors of it, but would it be appropriate to do something like this:

public Person(Dictionary dictionary, Object something)
{
}


and then cast something to either a string or string[] depending on whichever is appropriate?

Solution

Two things your could do:

-
Refactor the common code into a method (like Initialize) and call that:

private void Initialize()
{
    base.GetProperty("FirstName");
    base.GetProperty("LastName");
}

public Person(Dictionary dictionary, string someString)
    : base(dictionary, someString)
{
    Initialize();
}

public Person(Dictionary dictionary, string[] someStringArray)
    : base(dictionary, someStringArray)
{
    Initialize();
}


-
Not sure whether that's an option as I don't know how the behaviour changes but you could reduce one case to the other:

public Person(Dictionary dictionary, string someString)
    : this(dictionary, new [] { someString })
{
}

public Person(Dictionary dictionary, string[] someStringArray)
    : base(dictionary, someStringArray)
{
    base.GetProperty("FirstName");
    base.GetProperty("LastName");
}


Apart from that:

-
I'd avoid the casting. Apparently only strings or arrays of strings make sense to be passed in. If you change the parameter to object then it is no longer clear to the caller what he can and cannot pass in and would have to write a test to make sure that what he is passing in will be accepted. It reduces the clarity of the interface.

-
GetProperty seems to be a strange thing to call in a constructor. I'd expect it to return a property value yet you do nothing with the return value.

-
Consider making "FirstName" and "LastName" (and any other property string you use) string constants rather than literals - especially if you use them in more than one place. Lambda expressions might be an option if they are actual properties on the object.

Code Snippets

private void Initialize()
{
    base.GetProperty("FirstName");
    base.GetProperty("LastName");
}

public Person(Dictionary dictionary, string someString)
    : base(dictionary, someString)
{
    Initialize();
}

public Person(Dictionary dictionary, string[] someStringArray)
    : base(dictionary, someStringArray)
{
    Initialize();
}
public Person(Dictionary dictionary, string someString)
    : this(dictionary, new [] { someString })
{
}

public Person(Dictionary dictionary, string[] someStringArray)
    : base(dictionary, someStringArray)
{
    base.GetProperty("FirstName");
    base.GetProperty("LastName");
}

Context

StackExchange Code Review Q#32386, answer score: 7

Revisions (0)

No revisions yet.