CODEWITHSUNDEEP
c#datetime
CSharpNoob
CSharpNoob

Reputation: 1301

Datetime parameter for SQL Queries

I'm refactoring my code. Consider this example...

    public virtual List<Student> FetchEnrollmentList(DateTime admissionDateFrom, 
 DateTime admissionDateTo)
    {
        var students = new List<Student>();

            using (oconn = new OleDbConnection(OracleConnection))
            {
                oconn.Open();
                query = "SELECT * FROM Enrollment Where AdmissionDate between @AdmissionDateFrom and @AdmissionDateTo ";

                using (ocmd = new OleDbCommand(query, oconn))
                {
                    ocmd.Parameters.Add("@AdmissionDateFrom", OleDbType.Date).Value = admissionDateFrom;
                    ocmd.Parameters.Add("@AdmissionDateTo", OleDbType.Date).Value = admissionDateTo;

                    using (odr = ocmd.ExecuteReader())
                    {
                        while (odr.Read())
                            students.Add(new Student { Name = odr["StudentName"].ToString() });
                    }
                }
            }

        return students;
    }

I just want to eliminate the From and To so I created a type like this

     public virtual List<Student> FetchEnrollmentList(DateSpan admissionDate)
    {
        var students = new List<Student>();

        using (oconn = new OleDbConnection(OracleConnection))
        {
            oconn.Open();
            query = "SELECT * FROM Enrollment Where AdmissionDate between @AdmissionDateFrom and @AdmissionDateTo ";

            using (ocmd = new OleDbCommand(query, oconn))
            {
                ocmd.Parameters.Add("@AdmissionDateFrom", OleDbType.Date).Value = admissionDate.Start;
                ocmd.Parameters.Add("@AdmissionDateTo", OleDbType.Date).Value = admissionDate.End;

                using (odr = ocmd.ExecuteReader())
                {
                    while (odr.Read())
                        students.Add(new Student { Name = odr["StudentName"].ToString() });
                }
            }
        }

        return students;
    }

Is it OK to have it like this? Any other ideas? thanks....

Upvotes: 2

Views: 797

Answers (2)

BrokenGlass
BrokenGlass

Reputation: 160852

I agree with Martin, with just two DateTime parameters defining the boundaries of the list, there really is not much need to refactor, there simply is no "code smell" there yet.

On the other hand if you introduce other methods that as you say use several date range parameters I would refactor once I actually have those methods with DateSpan. Generally YAGNI until you really introduce those methods, only then should you refactor existing methods for uniformity imo.

I wouldn't introduce too much generality until there really is a need for it, refactoring is not about what you might need in some distant future, but what you can do about creating more readable and maintainable code with the code base you have and the features you want to add at the time.

Upvotes: 2

Marc Gravell
Marc Gravell

Reputation: 1062502

Re the "imagine I had 3" comment: then you have 3 ;) (or 3 pairs)

One alternative there (as the params increases) is to create a class that represents the query parameters, then there is only 1 formal parameter. Plus you can add options painlessly. For example:

public class StudentSearchRequest {
    public DateTime FooStart {get;set;}
    public DateTime FooEnd {get;set;}
    public bool PublicDataOnly {get;set;}
    public int SomethigElse {get;set;}
}

Upvotes: 1

Related Questions