How to avoid code repeating?

Question

I have the same code in method Output() in both classes Hour and Day. Is there way to avoid it to change code in one place instead of two?

class Program
{
    static void Main(string[] args)
    {
        Hour hour = new Hour("20150715 080000");
        Day day = new Day(hour);

        Console.WriteLine(String.Format("Hour: {0}", hour.Output()));
        Console.WriteLine(String.Format("Day: {0}", day.Output()));
    }
}

public interface IMoment
{
    string OutputMoment();
}

class Hour : IMoment
{
    public string Date;
    public string Time;

    public Hour (string s)
    {
        string[] parts = s.Split(';');
        this.Date = parts[0];
        this.Time = parts[1];
    }

    public string Output()
    {
        return Date + " " + Time;
    }
}

class Day : IMoment
{
    public string Date;
    public string Time;

    public Day(Hour hour)
    {
        this.Date = hour.Date;
        this.Time = hour.Time;
    }

    public string Output()
    {
        return Date + " " + Time;
    }

}

Answer 1

As an alternative to usr's answer, you could refactor your code to make the concern of writing the data to screen separate.

So your Hour and Day classes don't have 2 responsibilities (Single Responsibility Principle), as well as making the code much easier to update with more complex output functionality in the future, as you only have to change the code in the writer class. (or abstract it and create for e.g FileMomentWriter, etc)

public interface IMoment
{ 
   string MomentType {get;}
   string Date {get;set;}
   string Time {get;set;}
}


public class Hour:IMoment
{
    public string MomentType {get{return "Hour";}}
    public string Date {get;set;}
    public string Time {get;set;}

    public Hour (string s)
    {
        string[] parts = s.Split(';');
        this.Date = parts[0];
        this.Time = parts[1];
    }
}

public class Day: IMoment
{
    public string MomentType {get{return "Day";}}
    public string Date{get;set;}
    public string Time{get;set;}

    public Day(Hour hour)
    {
        this.Date = hour.Date;
        this.Time = hour.Time;
    }
}

public class ConsoleMomentWriter
{
   public void Write(IMoment moment)
   {
      Console.WriteLine("{0}: {1} {2}",moment.MomentType,moment.Date,moment.Time);
   }
}

class Program
{
    static void Main(string[] args)
    {
        Hour hour = new Hour("20150715 080000");
        Day day = new Day(hour);
        var writer = new ConsoleMomentWriter();
        writer.Write(hour);
        writer.Write(day);
    }
}

Answer 2

You should make an abstract class instead of an interface:

class Program
{
    static void Main(string[] args)
    {
        Hour hour = new Hour("20150715;080000");
        Day day = new Day(hour);

        Console.WriteLine(String.Format("Hour: {0}", hour.OutputMoment()));
        Console.WriteLine(String.Format("Day: {0}", day.OutputMoment()));
    }
}

public abstract class Moment
{
    public string Date;
    public string Time;

    public virtual string OutputMoment()
    {
        return Date + " " + Time;
    }

    public override string ToString()
    {
        return OutputMoment();
    }
}

class Hour : Moment
{
    public Hour(string s)
    {
        string[] parts = s.Split(';');
        this.Date = parts[0];
        this.Time = parts[1];
    }
}

class Day : Moment
{
    public Day(Hour hour)
    {
        this.Date = hour.Date;
        this.Time = hour.Time;
    }
}

Marking OutputMoment() as virtual will also allow you to over ride the default implementation, if you need to. I also overrode ToString() so that you can just do something like Console.WriteLine(hour); without having to call OutputMoment()

Answer 3

Don't make the mistake of creating a base class to share that method. That's a common misuse of inheritance. This technique breaks down in general and you introduce a meaningless class into the public interface of your class. Inheritance is not for code sharing. It's for "Liskov substitution".

Instead, create a static helper method, that takes both values as arguments and computes the result. This allows you to implement the formatting once. This is very easy to implement, works almost always and does not impact the public API of your classes. Don't fear the slightly larger syntax footprint. That's not a significant problem (most of the time).

Answer 4

just inherit a class that has that public member :)

class HasOutput
{
    public string Date;
    public string Time;

    public string Output()
    {
        return Date + " " + Time;
    }
}

class Hour : HasOutput, IMoment
{
    public Hour (string s)
    {
        string[] parts = s.Split(';');
        this.Date = parts[0];
        this.Time = parts[1];
    }
}

class Day : HasOutput
{
    public Day(Hour hour)
    {
        this.Date = hour.Date;
        this.Time = hour.Time;
    }
}