Kenmeister
Kenmeister

Reputation: 504

How to eliminate duplicate code

I have the following methods, that all return different types. I have four of such methods. In the spirit of good programming practices(DRY), should be using some OOP techniques such as inheritance or interfaces here or just roll with it. Any comments or code examples are welcome. Thank you.

  static AttendeeResponse GetAttendees(HttpWebRequest request)
    {
        HttpWebResponse resp = (HttpWebResponse)request.GetResponse();

        try
        {
            XmlSerializer ser = new XmlSerializer(typeof(AttendeeResponse));
            return (AttendeeResponse)ser.Deserialize(resp.GetResponseStream());
         }
        catch(Exception e)
        {
            error =  e.InnerException.ToString();
            return null;
        }

    }

    static MemberResponse GetMembers(HttpWebRequest request)
    {
        HttpWebResponse resp = (HttpWebResponse)request.GetResponse();

        try
        {
            XmlSerializer ser = new XmlSerializer(typeof(MemberResponse));
            return (MemberResponse)ser.Deserialize(resp.GetResponseStream());
        }
        catch (Exception e)
        {
            error = e.InnerException.ToString();
            return null;
        }

    }

Upvotes: 2

Views: 440

Answers (6)

djole351
djole351

Reputation: 11

You can eliminate duplicate code by delegates. for example: if I have to finish two task against database :

 OleDbCommand oci = new OleDbCommand(queryString, connection);
 oci.Connection.Open();
 oci.CommandText = "delete * from firstTable";
 oci.ExecuteNonQuery();
 oci.Connection.Close();

 OleDbCommand ocn = new OleDbCommand(queryString, connection);
 ocn.Connection.Open();
 ocn.CommandText = "select count(*) from secondTable";
 int affected = convert.ToInt32(ocn.ExecuteScalar();
 ocn.Connection.Close();

as you can see we have duplicated code here. So I need to refactor my code like this:

OleDbCommand myAccessCommand = null;
int count = -1;
OleDbWork(secondConnectionString, "SELECT COUNT(*) FROM   secondTable",
out myAccessCommand,
() =>
{
     count = Convert.ToInt32(myAccessCommand.ExecuteScalar());

});

OleDbWork(firstConnectionString, "delete * from firstTable",
out myAccessCommand,
() =>
{
    myAccessCommand.ExecuteNonQuery();
}

and my OleDbWork method will look like this:

internal bool OleDbWork(string connString, string command,
out OleDbCommand myAccessCommand, Action action)
    {
        OleDbConnection myAccessConn = null;
        myAccessCommand = null;
        try
        {
            myAccessConn = new OleDbConnection(connString);
        }
        catch (Exception)
        {
            MessageBox.Show("Cannot connect to database!");

            return false;
        }

        try
        {

            myAccessCommand = new OleDbCommand(command, myAccessConn);

            myAccessConn.Open();

            action();
            return true;

        }
        catch (Exception ex)
        {
            MessageBox.Show("Cannot retrieve data from database. \n{0}",
            ex.Message);

            return false;
        }
        finally
        {
            myAccessConn.Close();
        }
    }

I hope this will help

Upvotes: -1

Jon Skeet
Jon Skeet

Reputation: 1499730

How about:

// TODO: Improve the name :)
static T FetchItem<T>(HttpWebRequest request)
{
    using (HttpWebResponse resp = (HttpWebResponse)request.GetResponse())
    {
        try
        {
            XmlSerializer ser = new XmlSerializer(typeof(T));
            return (T) ser.Deserialize(resp.GetResponseStream());
        }
        catch (Exception e)
        {
            error = e.InnerException.ToString();
            return default(T);
        }
    }
}

Note that I've included a using statment to avoid you leaking connections. You don't need to close the stream as well, according to the docs.

The return null had to change to return default(T) in case T is a non-nullable value type; an alternative would be to restrict T to be a reference type using where T : class as part of the method declaration.

Use it like this:

MemberResponse members = FetchItem<MemberResponse>(request);
...
AttendeeResponse attendee = FetchItem<AttendeeResponse>(request);

Upvotes: 13

Kieran
Kieran

Reputation: 18049

MemberResponse membResp = (MemberResponse )StaticClassName.SerializeIt(request);

static Object SerializeIt(HttpWebRequest request)
    {
        HttpWebResponse resp = (HttpWebResponse)request.GetResponse();

        try
        {
            XmlSerializer ser = new XmlSerializer(typeof(MemberResponse));
            return (Object)ser.Deserialize(resp.GetResponseStream());
        }
        catch (Exception e)
        {
            error = e.InnerException.ToString();
            return null;
        }

    }

Upvotes: -1

Nestor
Nestor

Reputation: 13990

It looks like a good candidate for generics. Perhaps..?

static E GetPiece< E >(HttpWebRequest request)    {
{
   ...  
}

Upvotes: 0

David
David

Reputation: 19667

Use generics?

static T GetResponse<T>(HttpWebRequest request)
{
    HttpWebResponse resp = (HttpWebResponse)request.GetResponse();

    try
    {
        XmlSerializer ser = new XmlSerializer(typeof(T));
        return (T)ser.Deserialize(resp.GetResponseStream());
     }
    catch(Exception e)
    {
        error =  e.InnerException.ToString();
        return null;
    }

}

Upvotes: 1

Matt Warren
Matt Warren

Reputation: 10291

You could use Generics:

static T GetMembers<T>(HttpWebRequest request)
{
    HttpWebResponse resp = (HttpWebResponse)request.GetResponse();

    try
    {
        XmlSerializer ser = new XmlSerializer(typeof(T));
        return (T)ser.Deserialize(resp.GetResponseStream());
    }
    catch (Exception e)
    {
        error = e.InnerException.ToString();
        return null;
    }

}

Upvotes: 0

Related Questions