Examples of C# OO design for data access

Question

I am using the MS Enterprise Library 5.0 (data access blocks) for the main data layer of my application.

Although I understand the basics of OO (yes, I need to keep learning - contantly!), I am trying to work through understanding where/why/how to make use of good design, i.e. not repeating code un-necessarily etc, and also still trying to keep the code simple to read and of course debug.

So first of the bat, I have the following class and default/example method: ( p.s: the return value from the db/proc is XML )

public class ajaxget
{
    public enum outputType : int { JSON = 0, XML = 1 }

    public static string getMemberContacts(string sStartsWith, string sEndswith, outputType eOT)
    {
        //  Get the associated members based upon the criteria
        Database db = DatabaseFactory.CreateDatabase("MyDatabase");
        DbCommand cmd = db.GetStoredProcCommand("get_memberContactsXML");
        db.AddInParameter(cmd, "@memberID", DbType.Int64, Convert.ToInt64(sID));
        db.AddInParameter(cmd, "@startsWith1", DbType.String, sStartsWith);
        db.AddInParameter(cmd, "@startsWith2", DbType.String, sEndswith);
        IDataReader dr = db.ExecuteReader(cmd);
        StringBuilder sb = new StringBuilder();
        while (dr.Read())
        {
            sb.Append(dr.GetValue(0));
        }

        //  Clean up
        dr.Close();
        dr.Dispose();
        cmd.Dispose();

        //  What format to return?
        if (eOT == outputType.XML)
        {
            return sb.ToString();
        }
        if (eOT == outputType.JSON)
        {
            XmlDocument xdoc = new XmlDocument();
            xdoc.LoadXml(sb.ToString());
            return JsonConvert.SerializeXmlNode(xdoc);
        }
    }
}

So.. pretty basic so far. I am just trying to establish a template here of how I should proceed with say more "getMemberContacts" methods.. i.e. should I make a general "get" method which is a virtual method, and override the params? I know this sounds very remedial and it is! part of learning I guess.

So in summary, I am trying to of course re-use a "get" method, where the params/proc name is obviously different, however the actual return value (XML in this case, hence the while/append part that concatenates the returning XML blocks) needs to be the same, i.e. where the return can be left as XML or if supplied, JSON can be returned.

I realise this is very basic stuff/concept, but any pointers/ideas would be gratefully received!

David.

--- EDIT ---

Just to show the SQL 2008 XML part as I am curious about the comments about not returning XML straight from SQL - again I understand generally that getting the raw data in order to use it in different ways is a good thing, but in my case all of my front-end framework uses either XML or JSON (JS framework by the way is www.dhtmlx.com which has been awesome).

So the proc from SQL 2008 is:

CREATE PROCEDURE [dbo].[get_messagesForMemberXMLByID]
    @memberID as bigint=null,
    @days as int=-7
AS
BEGIN
    SET NOCOUNT ON;

    /* Setup the starting point (in the past) from when we wish to select the messages */
    Declare @startDate datetime
    set @startDate = DateAdd(d,@days, getdate())

    SELECT inboxID as "@id", convert(varchar(12),messageCreated,13) as messageCreated, convert(varchar(8), messageCreated,108) as messageCreatedTime, subject,message, messageOpened, messageFrom, messageFromID
    FROM bizzbox
    WHERE memberID = @memberID
    AND convert(char(8), messageCreated, 112) BETWEEN convert(char(8), @startDate,112) AND convert(char(8), getdate(), 112)
    ORDER BY messageCreated desc
    FOR XML PATH('row'), ROOT('rows')
END

Which the output pulls back as:

<rows>
  <row id="1">
    <messageCreated>31 Oct 2010 </messageCreated>
    <messageCreatedTime>21:27:32</messageCreatedTime>
    <subject>Testing 123</subject>
    <message>Wibble Bibble!</message>
    <messageFrom>David</messageFrom>
    <messageFromID>7</messageFromID>
  </row>
</rows>

.. which is exactly how I need the data formatted for my front-end.

I can see the reasons why not to use this method - i.e. what if I use another framework or need the data in a straight datatable for instance.. I guess I can have a param variable to output as XML or straight tabular form.. or dare I say even two procs.. but again, interested to hear the comments either way...

Thanks again for all the input - it really is very appreciated.

Answer 1

This is a pretty involved question and therefore answers to this will be many and varied. First of, since you're in the learning phase...

1. Stick to C# convention for names of methods, parameters, variables and such. And don't use Hungarian notation unless you really have to (there are some edge cases where Hungarian notation can be used but not where you're using them.
2. When using an Enum use a Switch statement instead of if-else.
3. also belonging to naming conventions point above, but I thought I'd highlight this one separately - the Enum values should be .Xml and .Json and not XML and JSON.

I have some questions: 1. Why are you using Xml? 2. Why does your database table that holds the member contacts not have "fields" that represent the attributes of a member's contacts?

To answer one of your question about re-using a "get" method.... I wouldn't I'd have seperate methods for each get because invariably the types and number of parameters will change. You may have common functionality within these methods made as private method that all "get" methods could use, but the each method itself should be seperate. This is because you should look at the data access class having a public (or internal) API. That is whe nyou look at this class's public methods, you should get a good sense of what this class can do and what parameters it requires in order to do it. Having one generic method will not only make you go through hoops trying to fit a square peg into a round hole but by looking at your public methods, you won't have a clue what functionality it provides.

Your method GetMemberContacts should have the following signature

public static List<MemberContact> GetMemberContacts(int memberId)

So in order to get a specific member's contacts that member's Id should be passed in as a parameter. The method should most likely return a list of Membercontact or a DataTabl/DataSet.

If you need Xml, then the business layer can take that data and convert it into xml. If you need to you can use the same List and convert it to Json. Let the business layer decide or better yet, let it have 3 methods that return a specific member's contacts in 3 different ways (A List, Json, Xml).

Not sure what sStartWith and sEndWith are in your code. Which means, you need a better method name and better parameters names that indicate the intent correctly.

Answer 2

First up, you need to use a using construct instead of manually disposing of things. You can use a using construct on anything that implements IDisposable:

StringBuilder sb = new StringBuilder(); 

using(DbCommand cmd = db.GetStoredProcCommand("get_memberContactsXML"))
{
    db.AddInParameter(cmd, "@memberID", DbType.Int64, Convert.ToInt64(sID)); 
    db.AddInParameter(cmd, "@startsWith1", DbType.String, sStartsWith); 
    db.AddInParameter(cmd, "@startsWith2", DbType.String, sEndswith); 
    using(IDataReader dr = db.ExecuteReader(cmd))
    { 
        while (dr.Read()) 
        { 
            sb.Append(dr.GetValue(0)); 
        } 
    }
}

The advantage of the using statement (besides being concise, readable and generally awesome) is that it will ensure that the resources are disposed of even if an exception occurs. In your example, an exception from the call to db.ExecuteReader(cmd) would have caused the command to not get disposed.

Shiv Kumar has also made some very good points. I'd do something like this:

public abstract class DataAccessLayerBase
{
    protected string GetXml(string storedProcedureName, OutputType outputType, params Tuple<string,DBType,object>[] parameters)
    {
        StringBuilder sb = new StringBuilder();

        //  Get the associated members based upon the criteria  
        Database db = DatabaseFactory.CreateDatabase("MyDatabase");  

        using(DbCommand cmd = db.GetStoredProcCommand(storedProcedureName))
        {
            foreach(var parameter in parameters)
            {
                db.AddInParameter(cmd, parameter.Item1, parameter.Item2, parameter.Item3);
            }

            using(IDataReader dr = db.ExecuteReader(cmd))
            {
                while (dr.Read())  
                {  
                    sb.Append(dr.GetValue(0));  
                }  
            }      

            switch(outputType)
            {
                case OutputType.Xml:
                    return sb.ToString();  
                case OutputType.Json:
                    XmlDocument xdoc = new XmlDocument();  
                    xdoc.LoadXml(sb.ToString());  
                    return JsonConvert.SerializeXmlNode(xdoc);  
                default:
                    throw new NotSupportedException(); // Some sort of error.
            }
        }
    } 
}

Then just create a subclass for each functional area:

public class MemberContactsDal : DataAccessLayerBase
{
    public string GetMemberContacts(long memberID, string startsWith, string endsWith, OutputType outputType)
    {
        // Call the method in the base class to handle all of the parsing.
        return GetXml(
            "get_memberContactsXML", 
            outputType, 
            new Tuple("@memberID", DBType.Int64, memberID ), 
            new Tuple("@startsWith1", DBType.String, startsWith), 
            new Tuple("@startsWith2", DBType.String, endsWith)
        );
    }
}

It does sound like there is something a bit supsicious going on in your stored procedures if they are returning XML though...

I hope that my example helps. I haven't tried to compile it, but it should be about right.