Pergunta

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.

Foi útil?

Solução

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.

Outras dicas

It is not so much a code smell as it is an operation smell. I think you should ask those "different areas of the organisation" to agree on a company wide naming convention before you proceed. You are trying to cater to their failure to communicate and so make it harder to ever straighten this legacy out.

Put a term on it, give them two weeks to get back to you. Which they probably won't. Then choose whatever names you like and declare those to be the company standard.

I think you should not have alias properties but pick one, perhaps the most common and document the aliases in a xml comment:

/// <summary>
/// Gets person's first name also known as Forename.
/// </summary>
public string FirstName { get; set; }

If everyone would use multiple properties for the same thing we would have arrays and lists with such properties as:

public int Count { get; }
public int NumberOfItems { get; }
public int Length { get; }

It's not only difficult to maintain but also the learning curve is steeper. You need to know all of them to know that they actually are identical. If someone would like to use an alias anyway, he can always write his own extension.

As an alternative, one could still have the single class approach with your names as well as single properties, but provide a generic accessor using reflection:

public static T GetPropertyValue<T>(object obj, string propName) 
{ 
    return (T)obj.GetType().GetProperty(propName).GetValue(obj, null); 
}

In front of that one will need a lookup configuration(xml, json, etc) so that items like:

FamilyName

get transformed to:

SurName

Then Surname is passed into the reflective method. So, the naming aliases are stored in a configuration as opposed to code or additional classes. If new aliases are added, just update the "alias config".

Either access the property directly or use the more convoluted approach of letting the caller try and access the property using their own name the code will do the look and pull the value using the reflective method.

This keeps your code base clean. Keep in mind reflection access versus directly access will be slower.

Licenciado em: CC-BY-SA com atribuição
scroll top