Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

I'm trying to implement the Strategy Pattern into my current code base. I would like to know if i'm going in the right direction on this?

IOracleDB.cs:

/// <summary>
/// Strategy
/// </summary>
interface IOracleDB
{
    DataSet DatabaseQuery(string query, OracleConnection oracleConnection);
    DataSet DatabaseQuery(string procedure, string parameters, OracleConnection oracleConnection);
}

OracleDBContext.cs:

/// <summary>
/// Context
/// </summary>
class OracleDBContext
{
    private readonly string _EmailConnectionString = ConfigurationManager
        .ConnectionStrings["EmailConnectionString"].ConnectionString;

    private readonly string _PhConnectionString = ConfigurationManager
        .ConnectionStrings["PhConnectionString"].ConnectionString;

    private readonly IOracleDB _oracleDB;
    private readonly OracleConnection _oracleConnection;

    public OracleDBContext(IOracleDB oracleDB, string table)
    {
        _oracleDB = oracleDB;

        switch (table.ToUpper())
        {
            case "Email":
                _oracleConnection = new OracleConnection(_EmailConnectionString);
                return;

            case "Ph":
                _oracleConnection = new OracleConnection(_PhConnectionString);
                return;

            default:
                return;
        }
    }

    public void ConnectToDatabase()
    {
        _oracleConnection.Open();
    }

    public void DisconnectFromDatabase()
    {
        _oracleConnection.Close();
        _oracleConnection.Dispose();
    }

    public string ConnectionStatus()
    {
        if (_oracleConnection != null)
        {
            return _oracleConnection.State.ToString();
        }
        return "OracleConnection is null.";
    }

    public DataSet DatabaseQuery(string query)
    {
        return _oracleDB.DatabaseQuery(query, _oracleConnection);
    }

    public DataSet DatabaseQuery(string procedure, string parameters)
    {
        return _oracleDB.DatabaseQuery(procedure, parameters, _oracleConnection);
    }
}

EmailTableClass.cs:

/// <summary>
/// Concrete Strategy
/// </summary>
class EmailTableClass : IOracleDB
{

    public DataSet DatabaseQuery(string query, OracleConnection oracleConnection)
    {
        DataSet dataSet = new DataSet();
        OracleCommand cmd = new OracleCommand(query);
        cmd.CommandType = CommandType.Text;
        cmd.Connection = oracleConnection;

        using (OracleDataAdapter dataAdapter = new OracleDataAdapter())
        {
            dataAdapter.SelectCommand = cmd;
            dataAdapter.Fill(dataSet);
        }

        return dataSet;
    }

    public DataSet DatabaseQuery(string procedure, string parameters, OracleConnection oracleConnection)
    {
        throw new NotImplementedException();
    }
}

PhTableClass.cs:

/// <summary>
/// Concrete Strategy
/// </summary>
class PhTableClass : IOracleDB
{

    public DataSet DatabaseQuery(string query, OracleConnection oracleConnection)
    {
        throw new NotImplementedException();
    }

    public DataSet DatabaseQuery(string procedure, string parameters, OracleConnection oracleConnection)
    {
        DataSet dataSet = new DataSet();
        OracleCommand cmd = new OracleCommand
        {
            CommandText = procedure,
            CommandType = CommandType.StoredProcedure,
            Connection = oracleConnection,
        };

        OracleParameter oracleParameter = new OracleParameter
        {
            ParameterName = parameters
        };

        using (OracleDataAdapter dataAdapter = new OracleDataAdapter())
        {
            dataAdapter.SelectCommand = cmd;
            dataAdapter.Fill(dataSet);
        }

        return dataSet;
    }
}
share|improve this question
Thanks to everyone who responded to my question. I actually answered my own question a long time ago. I ended up rewriting everything and it all worked out in the end. Lol I don't know what I was thinking at the time. I blame it on lack of sleep. – Eric Anderson Sep 12 '11 at 20:28

2 Answers

up vote 3 down vote

There are several issues that could be improved:

  1. First of all, having concrete classes which are all tied to a specific database (in this case Oracle), is usually completely opposite of what a Data Layer's responsibility should be. Consider changing the classes' names as follows:

    • OracleConnection --> DbConnection
    • OracleCommand --> DbCommand
    • OracleParameter --> DbParameter.

    To instantiate a new command, don't use a specific constructor, but rather use the DbConnection.CreateCommand() method (and, likewise, DbCommand.CreateParameter() to create its parameters). That means you only need to change the actual connection instance, if you decide to switch from Oracle to a different db provider one day.

  2. Next, strategy pattern is usually used to abstract a functionality which several different algorithms can execute, while exposing a single (common) interface. Creating a different strategy for different table queries is not appropriate, because the calling code already knows which table it is querying (there are no additional strategical choices your code can do).

  3. I am not sure if the NotImplementedExceptions are thrown deliberately, or you really haven't yet implemented those methods. If you are throwing to indicate that a certain implementation doesn't support that functionality, that is another alarm that these two implementations are not really different strategies for the same task.

share|improve this answer
up vote 0 down vote

The design is wrong.
Let's say we have a class:

    public class SomeClass
    {
        public void DoSomethingWithDb(OracleDBContext dbContext)
        {
            dbContext.ConnectToDatabase();
            var dataSet = dbContext.DatabaseQuery("some procedure", "some parameters");
            dbContext.DisconnectFromDatabase();
        }
    }

What will we get when we use this class like this:

        var dbContext = new OracleDBContext(new EmailTableClass(), "Email");
        var someClass = new SomeClass();
        someClass.DoSomethingWithDb(dbContext);

We'll get NotImplementedException.
By the way, judging from the names of the classes, I can assume that we'll get an error if we write:

var dbContext = new OracleDBContext(new EmailTableClass(), "Ph");

I would:
1. Make EmailTableClass: OracleDBContext and PhTableClass: OracleDBContext
2. Remove method DatabaseQuery(string query, OracleConnection oracleConnection) from PhTableClass and method DatabaseQuery(string procedure, string parameters, OracleConnection oracleConnection) from EmailTableClass.
3. Move names of the tables to EmailTableClass and PhTableClass.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.