views:

194

answers:

7

How would you refactor these two classes to abstract out the similarities? An abstract class? Simple inheritance? What would the refactored class(es) look like?

public class LanguageCode
{
    /// <summary>
    /// Get the lowercase two-character ISO 639-1 language code.
    /// </summary>
    public readonly string Value;

    public LanguageCode(string language)
    {
        this.Value = new CultureInfo(language).TwoLetterISOLanguageName;
    }

    public static LanguageCode TryParse(string language)
    {
        if (language == null)
        {
            return null;
        }

        if (language.Length > 2)
        {
            language = language.Substring(0, 2);
        }

        try
        {
            return new LanguageCode(language);
        }
        catch (ArgumentException)
        {
            return null;
        }
    }
}

public class RegionCode
{
    /// <summary>
    /// Get the uppercase two-character ISO 3166 region/country code.
    /// </summary>
    public readonly string Value;

    public RegionCode(string region)
    {
        this.Value = new RegionInfo(region).TwoLetterISORegionName;
    }

    public static RegionCode TryParse(string region)
    {
        if (region == null)
        {
            return null;
        }

        if (region.Length > 2)
        {
            region = region.Substring(0, 2);
        }

        try
        {
            return new RegionCode(region);
        }
        catch (ArgumentException)
        {
            return null;
        }
    }
}
+2  A: 

It depends, if they are not going to do much more, then I would probably leave them as is - IMHO factoring out stuff is likely to be more complex, in this case.

Chris Kimpton
A: 

This is a rather simple question and to me smells awefully like a homework assignment.

You can obviously see the common bits in the code and I'm pretty sure you can make an attempt at it yourself by putting such things into a super-class.

PintSizedCat
A: 

You could maybe combine them into a Locale class, which stores both Language code and Region code, has accessors for Region and Language plus one parse function which also allows for strings like "en_gb"...

That's how I've seen locales be handled in various frameworks.

Ben
A: 

These two, as they stand, aren't going to refactor well because of the static methods.

You'd either end up with some kind of factory method on a base class that returns an a type of that base class (which would subsequently need casting) or you'd need some kind of additional helper class.

Given the amount of extra code and subsequent casting to the appropriate type, it's not worth it.

Steve Morgan
A: 

I'm sure there is a better generics based solution. But still gave it a shot.

EDIT: As the comment says, static methods can't be overridden so one option would be to retain it and use TwoLetterCode objects around and cast them, but, as some other person has already pointed out, that is rather useless.

How about this?

public class TwoLetterCode {
    public readonly string Value;
    public static TwoLetterCode TryParseSt(string tlc) {
        if (tlc == null)
        {
            return null;
        }

        if (tlc.Length > 2)
        {
            tlc = tlc.Substring(0, 2);
        }

        try
        {
            return new TwoLetterCode(tlc);
        }
        catch (ArgumentException)
        {
            return null;
        }
    }
}
//Likewise for Region
public class LanguageCode : TwoLetterCode {
    public LanguageCode(string language)
    {
        this.Value = new CultureInfo(language).TwoLetterISOLanguageName;
    }
    public static LanguageCode TryParse(string language) {
        return (LanguageCode)TwoLetterCode.TryParseSt(language);
    }
}
Vinko Vrsalovic
Unfortunately, this isn't going to work well because your TryParse method is not static. TryParse is effective a factory method that returns a new LanguageCode or RegionCode object. In your example, you need to create an object first. You can't override the base TryParse because it's static.
Steve Morgan
Hmm, unfortunately, a new instance of TwoLetterCode cannot be created because it's abstract (see: return new TwoLetterCode(tlc)). I can remove "abstract" from TwoLetterCode's class delcaration, but I don't want that--only protected types should be able to create a TwoLetterCode.
Chris
A: 
  1. Create a generic base class (eg AbstractCode<T>)
  2. add abstract methods like

    protected T GetConstructor(string code);
    
  3. override in base classes like

    protected override RegionCode GetConstructor(string code)
    {
        return new RegionCode(code);
    }
    
  4. Finally, do the same with string GetIsoName(string code), eg

    protected override GetIsoName(string code)
    {
        return new RegionCode(code).TowLetterISORegionName;
    }
    

That will refactor the both. Chris Kimpton does raise the important question as to whether the effort is worth it.

Steve Cooper
A: 

Unless you have a strong reason for refactoring (because you are going to add more classes like those in near future) the penalty of changing the design for such a small and contrived example would overcome the gain in maintenance or overhead in this scenario. Anyhow here is a possible design based on generic and lambda expressions.

public class TwoLetterCode<T>
{
    private readonly string value;

    public TwoLetterCode(string value, Func<string, string> predicate)
    {
     this.value = predicate(value);
    }

    public static T TryParse(string value, Func<string, T> predicate)
    {
     if (value == null)
     {
      return default(T);
     }

     if (value.Length > 2)
     {
      value = value.Substring(0, 2);
     }

     try
     {
      return predicate(value);
     }
     catch (ArgumentException)
     {
      return default(T);
     }
    }

    public string Value { get { return this.value; } }
}

public class LanguageCode : TwoLetterCode<LanguageCode>  {
    public LanguageCode(string language)
     : base(language, v => new CultureInfo(v).TwoLetterISOLanguageName)
    {
    }

    public static LanguageCode TryParse(string language)
    {
     return TwoLetterCode<LanguageCode>.TryParse(language, v => new LanguageCode(v));
    }
}

public class RegionCode : TwoLetterCode<RegionCode>
{
    public RegionCode(string language)
     : base(language, v => new CultureInfo(v).TwoLetterISORegionName)
    {
    }

    public static RegionCode TryParse(string language)
    {
     return TwoLetterCode<RegionCode>.TryParse(language, v => new RegionCode(v));
    }
}
smink