views:

56

answers:

3

Hi,

I have a class as follows :-

interface IFilterCondition
{
    List<Name> ApplyFilter(List<Name> namesToFilter);
}

class FilterName : IFilterCondition
{
    public NameFilterEnum NameFilterEnum{ get; set; }

    public List<Name> ExcludeList { get; set; }

    public char StartCharacter{ get; set; }

    #region IFilterCondition Members

    public List<Name> ApplyFilter(List<Name> namesToFilter)
    {
        switch (NameFilterEnum)
        {
            case NameFilterEnum.FilterFirstName:
                // Check Exclude List
                // Check Start Character
                break;
            case NameFilterEnum.FilterLastName:
                // Check Exclude List only
                break;
            default:
                break;
        }
        return namesToFilter;
    }

    #endregion
}

enum NameFilterEnum
{
    None,
    FilterFirstName,
    FilterLastName
}

Note that only if it is flagged as a FilterFirstName then it will require the StartCharacter property.

Is the above correct or should I separate out FirstName filter and LastName filter as they require different properties? Coz I think in this instance, some business rules needs to be enforced when inputing data to this class.

Please advice, Thanks

+2  A: 

Enumerations are often a code smell that your design isn't quite right. In this case you could do better by refactoring the exclusion list functionality into a base class and then adding separate derived classes for the first and last name filters. The last name filter wouldn't do anything different from the generic filter but the first name filter would also check the start character.

interface IFilterCondition
{
    List<Name> ApplyFilter(List<Name> namesToFilter);
}

abstract class FilterName : IFilterCondition
{
    public List<Name> ExcludeList { get; set; }

    public virtual List<Name> ApplyFilter(List<Name> namesToFilter)
    {
        // Check Exclude List
        return namesToFilter;
    }
}

class FilterFirstName : FilterName
{
    public char StartCharacter{ get; set; }

    public override List<Name> ApplyFilter(List<Name> namesToFilter)
    {
        namesToFilter = base.ApplyFilter(namesToFilter);

        // Check Start Character
        return namesToFilter;
    }
}

class FilterLastName : FilterName
{
}
John Kugelman
So in terms of database design, I have to store each object as an individual table then? Like firstnamefilter table and lastnamefilter table?
Titan
No, I would go with an enum-type approach for your database. It would be fine in that context.
John Kugelman
+1  A: 

Looking at what you've got there it seems like it would make the most sense to have multiple classes which inherit from IFilterCondition defined that each fully implement their own version of ApplyFilter()- FirstNameFilter, LastNameFilter, PhoneNumberFilter, etc.

To save code, you can simply derive from a concrete implementation you create if and when you need to reuse similar logic to define the filter. For example you might have an [abstract] StartCharacterFilter that defines that character and truncates the list in its ApplyFilter() method then, FirstNameFilter would simply override ApplyFilter(), call the base implementation and pass the result to its own logic.

Nathan Taylor
+1  A: 

This seems like improper factoring. The filter apparently is strongly coupled with an enum value, and the type determines what specific criteria it needs, and the data values for that are jumbled into a single class.

It is more "pure" to have separate filter classes with only relevant data contained within each. The enum can probably be removed if you make that change.

bobbymcr