Two classes that behave identically yet are semantically different

Question

I am writing a program which is a similar to Ruby's Active Record Migrations, in which that every migration has both an "Up" and "Down" in terms of creating a change to the database, "Up" meaning bring the database forward in time, and "Down" reversing that change.

My program has two main functions to achieve its use-cases

1. Get a list of scripts off the filesystem
2. Execute the appropriate migrations on the database

The "Up" and "Down" migrations are stored in their own separate files, but they match with a similar naming scheme

• 2014000000000001_CreateTable_up.sql
• 2014000000000001_CreateTable_down.sql

There is a 1:1 correlation between up and down scripts. Having just an up without the corresponding down script is not valid (and vise verse).

I have two classes in my project to represent these filesystem files.

public sealed class UpScript // (and DownScript, which looks identical)
{
    public UpScript(long version, string name, string content)
    {
        Content = content;
        Version = version;
        Name = name;
    }

    public long Version { get; private set; }
    public string Name { get; private set; }
    public string Content { get; private set; }
}

The reason that I did not make a single class called Script is I didn't want to create ambiguity about the purpose of a specific Script instance. UpScript and DownScript classes should not be used interchangeability as they do not conform to the Liskov Substitution Principle.

The problem that I have here is a few parts of code that are dealing with both either UpScript or DownScript instances look almost identical. How do I reduce the duplication without losing the expressiveness of having different classes?

My current ideas with concerns are:

Abstract base class

public abstract class Script
{
    protected Script(long version, string name, content)
    {
        // code
    }

    // properties
}

public sealed UpScript : Script
{
    public UpScript(long version, string name, string content)
        : this(version, name, content)
    {}
}

public sealed DownScript : Script
{
    public DownScript(long version, string name, string content)
        : this(version, name, content)
    {}
}

This reduces the redundant code in the two classes, however, code that creates and consumes is still very duplicated.

Single class with Direction property

public enum Direction { Up, Down }

public sealed class Script
{
    protected Script(Direction direction, long version, string name, string content)
    {
        // ...
    }

    public Direction Direction { get; private set; }

    // ...
}

This code reduces the duplication, however, it adds onus on consumes to ensure the proper script is passed to it and it creates ambiguity if you ignore the Direction property.

Single class with two Content properties

public sealed class Script
{
    protected Script(long version, string name, string upContent, string downContent)
    {
        // ...
    }

    public string UpContent { get; private set; }
    public string DownContent { get; private set; }
}

This reduces duplication and is not ambiguous, however, the usage of the program is to either run the up script, or run the down scripts. With this scheme, the code that gathers and creates these Script instances is doing twice as much work, I don't consider not doing this being premature optimization because if the user wants to do an up migration, there's no point is looking at down scripts.

With all that said I may be looking at the wrong problem to solve entirely.

If more information would be helpful in forming a suggestion, you can view ScriptMigrations on GitHub

Answer 1

My C# is rusty, hopefully I haven't glossed over any language constraints.

I would implement UpScript and DownScript as interfaces that have the same members:

public interface IUpScript {
    public long Version { get; private set; }
    public string Name { get; private set; }
    public string Content { get; private set; }
}
// ...
public interface IDownScript {
    public long Version { get; private set; }
    public string Name { get; private set; }
    public string Content { get; private set; }
}

This approach attempts to provide the minimal code needed to create a distinction between up and down, nothing more. Thoughts:

• Classes implementing an interface will have the common properties you've defined as in your other approaches but don't inherit implementation details like a constructor.
• Your implementing classes would be free to extend other classes if they needed.
• It looks as though you don't need to support multiple implementations of up and down scripts but you'd have no trouble doing so.

Answer 2

I think your solution with the abstract base class is just fine. It's common OO programming: you have an abstract type with two different implementations. You should not worry about about duplication of creating two classes. In a language like Scala that provides some syntactic facilities, you can define such cases in a more concise way, but for C# and Java I don't think you can do much better. In Scala you can use case classes for example:

trait Script(long version, string name, content)
case class UpScript(long version, string name, content) extends Script
case class DownScript(long version, string name, content) extends Script

About instantiation, in your current case it's just fine as you have only two subclasses at the moment. You can think of having factories and then when reading the up files, use UpFactory, say, and vice versa. This way the client code will not be bound to the actual implementation.

Answer 3

The solution with the abstract base class is the way to go.

To reduce duplication in the code that determines which scripts to process, you can introduce a factory class/method that creates either a UpScript or a DownScript instance based on a parameter (the same parameter that determines if the scripts with the "_up" or the "_down" suffix should be used).

If the execution order of the scripts depends on the direction, you can add a comparison method to Script (with implementations in UpScript and DownScript) to correctly order the Script instances.
Execution of the scripts then becomes as easy as iterating over an ordered collection without needing to know if you are upgrading or downgrading.

Answer 4

How do I reduce the duplication without losing the expressiveness of having different classes?

By considering the scenarios in which your program interacts with the scripts, as stated in your top-level description:

1. Get a list of scripts off the filesystem
2. Execute the appropriate migrations on the database

The first decision is which up or down scripts to apply during migration:

public interface IScriptSource
{
    IEnumerable<IScript> ReadUpScripts();

    IEnumerable<IScript> ReadDownScripts();
}

The second decision is to execute each script, which will move the database forward or backward in time:

public interface IScript
{
    void Execute();
}

This gives us a nice basis for addressing your primary concern:

The problem that I have here is a few parts of code that are dealing with both either UpScript or DownScript instances look almost identical.

With these top-level abstractions in place, we can shape implementations around shared details:

public abstract class Script : IScript
{
    protected Script(long version, string name, string content)
    {
        Version = version;
        Name = name;
        Content = content;
    }

    protected long Version { get; private set; }
    protected string Name { get; private set; }
    protected string Content { get; private set; }

    public virtual void Execute()
    {
        SomethingShared();

        MoveInDirection();

        SomethingElseShared();
    }

    protected abstract void MoveInDirection();
}

Then differentiate the specifics of each type:

public sealed class UpScript : Script
{
    public UpScript(long version, string name, string content)
        : base(version, name, content)
    {}

    protected override void MoveInDirection()
    {
        // Up-specific
    }
}

public sealed class DownScript : Script
{
    public DownScript(long version, string name, string content)
        : base(version, name, content)
    {}

    protected override void MoveInDirection()
    {
        // Down-specific
    }

    protected override SomethingElseShared()
    {
        base.SomethingElseShared();

        // Down-specific
    }
}

This will help consolidate the spread-out code by pairing it directly with the data.

It also gets the Liskov stamp of approval due to IScriptSource hiding the concrete types.