C# – Identifying Code Smells in Properties and Methods

anti-patternsccode smellproperties

I'm trying to ascertain whether the use of multiple references to the same property is code smell / an anti-pattern, based on the needs of the organisation.

As an example, consider:

abstract class Person {

    public string Title { get; set; }
    public string Forename { get; set; }
    public string MiddleNames { get; set; }
    public string Surname { get; set; }

    public string GivenName  { get { return Forename; } set { Forename = value; } }
    public string FamilyName { get { return Surname; }  set { Surname = value; } }
    public string FirstName  { get { return Forename; } set { Forename = value; } }
    public string LastName   { get { return Surname; }  set { Surname = value; } }

}

As you can see, I have several other properties, (kind of) aliasing or giving alternate names to properties in the object, but basically repeating exactly the functionality of the base property without exception.

The reasons behind this is down to the naming conventions utilised in different areas of the organisation; it's providing multiple ways of addressing the same information, allowing each different area to use their preferred method of access. Because each of the other properties reference the base property, any programmatic changes only need to be included in the said base property.

And so to clarify…

@GregBurghardt has kindly posted alternatives to my conundrums, but I feel that I must qualify one of my statements a little more.

In VB.NET I can quite happily code interfaces into my classes but use alternative names in the actual member implementation, like so…

Public Interface IPerson
    Property Forename() As String
    Property Surname() As String
End Interface

Public Class BillingPerson
    Implements IPerson

    Public Property GivenName() As String Implements IPerson.Forename
    ...
    Public Property FamilyName() As Int32 Implements IPerson.Surname
    ....
End Class

Notice, here, that I've used a different property name for the (VB) property, but still implemented the true name of the interface (e.g. Public Property GivenName () As String Implements IPerson.Forename). This means that I can reference my BillingPerson class, despite the obvious property name differences, using the interface…

'Define our interface type object here...
Dim myP As IPerson = New BillingPerson()
'Assign values directly to the interface members...
myP.Forename = "Fred"
myP.Surname = "Bloggs"
'Output the interface members...
Console.WriteLine(myP.Forename & " " & myP.Surname)

…or I can refer to the original class directly….

'Define our BillingPerson object here...
Dim myP As BillingPerson
'Assign values directly to the interface members...
myP.GivenName = "Fred"
myP.FamilyName = "Bloggs"
'Output the interface members...
Console.WriteLine(myP.Forename & " " & myP.Surname)

Using this methodology, my problem would be very short-lived , and I'd be able to create as many classes based on the interface as I need.

This doesn't appear to work the same way in C#, however, where I can't directly alias the name of the interface members.

Best Answer

Since your organization can't seem to agree on naming things, it feels like you need to architect your application assuming there will be lots of things they don't agree on. It sounds like it will only get worse.

Maybe what you need instead is to adopt more of a facade or adapter pattern.

So, here is your "standard" person:

public class Person
{
    public string Title { get; set; }
    public string Forename { get; set; }
    public string MiddleNames { get; set; }
    public string Surname { get; set; }

    // Enterprise wide behavior goes here
}

And the Billing Department can have its own way:

public class BillingDepartmentPerson
{
    private readonly Person person;

    public string GivenName
    {
        get { return person.Forename; }
        set { person.Forename = value; }
    }

    public string FamilyName
    {
        get { return person.Surname; }
        set { person.Surname = value; }
    }

    public string FirstName
    {
        get { return person.Forename; }
        set { person.Forename = value; }
    }

    public string LastName
    {
        get { return person.Surname; }
        set { person.Surname = value; }
    }

    public BillingDepartmentPerson(Person person)
    {
        this.person = person;
    }

    // Billing department specific behavior goes here
}

And so can the Shipping Department:

public class ShippingDepartmentPerson
{
    private readonly Person person;

    public string Title
    {
        get { return person.Title; }
        set { person.Title = value; }
    }

    public string Forename
    {
        get { return person.Forename; }
        set { person.Forename = value; }
    }

    public string MiddleNames
    {
        get { return person.MiddleNames; }
        set { person.MiddleNames = value; }
    }

    public string Surname
    {
        get { return person.Surname; }
        set { person.Surname = value; }
    }

    public ShippingDepartmentPerson(Person person)
    {
        this.person = person;
    }

    // Shipping department specific behavior goes here
}

Yes, there is some repetition of code, but now you have a way of encapsulating the "organization area specific" functionality and separate it from the common, enterprise wide functionality in the Person class.

It also means you can't accidentally invoke operations specific to the billing department in the context of the shipping department. It gives you a nice Separation of Concerns so refactoring these different areas can be isolated, and the effects of any refactoring jobs can be limited in scope (see also the Open/Closed Principal).

Paul commented:

if this had been VB.NET, it wouldn't have been a problem, I could have used an interface and aliased all the member names. But it isn't!

... in VB.NET I can do something like: Public Function x() As Boolean Implements IY.a. I haven't seen anything in C# to allow this.

C# does:

public class MyItems : IEnumerable<int>
{
    public IEnumerator<int> GetEnumerator()
    {
        throw new NotImplementedException();
    }

    // Implement an interface method explicitly
    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        throw new NotImplementedException();
    }
}

VB.NET and C# compile down to the same exact MSIL code that gets executed in the same exact Common Language Runtime.

C# and VB.NET, as long as you compare Visual Studio and .NET framework versions, support the same features.

Related Topic