Get item from database by ID and get all items from database

Question

I have 2 methods for getting data from the database (the data I get are both List of tuples with doubles and datetimes in it). I would like to know if it is possible to use one method for both queries.

How can I make this code DRY? In my opinion I'm defining 2 methods which clearly does nearly the same. How can I improve this? I've read some about predicates and delegates but I'm not sure how to apply them here.

 private IEnumerable<Tuple<double, DateTime>> GetCarHdopById(int id)
        {
            var tp = new List<Tuple<double, DateTime>>();
            db.OpenConnection();
            var result = db.QueryDatabaseById("SELECT avg(hdop),to_char(datetime, 'YYYY-MM-DD') as average_hdop FROM positions where unitid = :id group by to_char(datetime, 'YYYY-MM-DD');",id);
            while (result.Read())
            {

                var avgHdopPerCar = Convert.ToDouble(result[0]);
                var date = Convert.ToDateTime(result[1]);
                tp.Add(new Tuple<double, DateTime>(avgHdopPerCar, date));
            };
            db.CloseConnection();
            return tp;

        }

        private List<Tuple<double, DateTime>> GetCarHdopTotalCarsPerDay()
        {
            var tp = new List<Tuple<double, DateTime>>();
            db.OpenConnection();
            var result = db.QueryDatabase("SELECT avg(hdop),to_char(datetime, 'YYYY-MM-DD') as average_hdop FROM positions group by to_char(datetime, 'YYYY-MM-DD');");
            while (result.Read())
            {

                var avgHdopPerCar = Convert.ToDouble(result[0]);
                var date = Convert.ToDateTime(result[1]);
                tp.Add(new Tuple<double, DateTime>(avgHdopPerCar, date));
            };
            db.CloseConnection();
            return tp;
        }

Query database method (I can probably overload this one):

   public NpgsqlDataReader QueryDatabase(string query)
    {
        var command = new NpgsqlCommand(query, conn);
        command.Connection = conn;
        return command.ExecuteReader();
    }

     public NpgsqlDataReader QueryDatabaseById(string query, int parameters)
    {
        var command = new NpgsqlCommand(query, conn);
        command.Parameters.Add("id",NpgsqlDbType.Integer);
        command.Prepare();
        command.Parameters[0].Value = parameters;
        command.Connection = conn;
        return command.ExecuteReader();
    }

Answer 1

First some general information. Your code currently solves a specific problem of the "object relational impedance mismatch" in a specific way. Consider to minimize the effort and complexity to handle this mismatch by using an object-relational mapping framework. Here is the thing: Every single step of code improvement code will lead towards an implementation of an more generalized object-relational mapper anyway. But that will be a hard way to go. Every answer given will be one of those:

1. Write your own OR-Mapper
2. Use an existing OR-Mapper
3. Develop your code towards an OR-Mapper
4. Ignore the problem through adding specific code that makes your solution more inflexible

I don't think that 1. or 2. will be covered here. I will ignore 4 because we are here to improve code. Therefore I will only provide some steps of 3.

The first step to remove redundancy is to identifiy the parts that need to be flexible. In your case you may need a new class of objects that holds following information:

1. table column name (unitid)
2. the type of the table column (NpgsqlDbType.Integer)
3. value to query (id)
4. the placeholder within the query (":id")

So what you have built then is something like a QueryParameter for a specific table (average_hdop).

Now you can generalize your parameter of the query-method:

private List<Tuple<double, DateTime>> GetCarHdops(List<QueryParameter> queryParams)

With this information you will be able to built your where-clause dynamically through string concatenation. This will be some effort and error prone. Furthermore you are currently restricted to one conjunction. You have to decide between "and" or "or". And make sure to omit the where-clause in the whole if the queryParams are empty.

The parameter will be passed to the method "QueryDatabase" as well. There you can built your "NpgsqlCommand" dynamically.

This is only a suggestion to develop the code in the "right" direction. As you already asked to avoid redundancy on this way you also gain flexibility.

The flexibility now ends at the level where you want to built queries with "and" and "or". If you want to do that you must have somethig like the Criteria-API in JPA (Java). But as I said I won't cover that in this answer.

One other thing you should think about: db.OpenConnection(); and db.CloseConnection(); are perfect candidates be extracted to a template. (see https://en.wikipedia.org/wiki/Template_method_pattern)

Answer 2

You could make the int parameter in the first call as a nullable integer which would allow you to use one method for both:

    private IEnumerable<Tuple<double, DateTime>> GetCarHdopById(int? id)
    {
        var tp = new List<Tuple<double, DateTime>>();
        db.OpenConnection();
        var result = db.QueryDatabaseById("SELECT avg(hdop),to_char(datetime, 'YYYY-MM-DD') as average_hdop FROM positions where unitid = :id group by to_char(datetime, 'YYYY-MM-DD');",id);
        while (result.Read())
        {

            var avgHdopPerCar = Convert.ToDouble(result[0]);
            var date = Convert.ToDateTime(result[1]);
            tp.Add(new Tuple<double, DateTime>(avgHdopPerCar, date));
        };
        db.CloseConnection();
        return tp;

    }

And in your query method, QueryDatabaseById, perform a check if the id parameter has a value:

    public NpgsqlDataReader QueryDatabaseById(string query, int? parameters)
    {
       var command = new NpgsqlCommand(query, conn);

       if(parameters.HasValue) 
       { 
          command.Parameters.Add("id",NpgsqlDbType.Integer);
          command.Prepare();
          command.Parameters[0].Value = parameters;            

        }

       command.Connection = conn;
       return command.ExecuteReader();
}