1

Currently I return everything from my Repo as a List. I am looking to change these to IQueryable just so people can refine the results and not suffer from another sql request(I am using nhibernate).

I have a question though to make life easier on everyone I have something like this in my repo

    public List<CalendarAppointment> GetAppointment(Student student, DateTime start, DateTime end)
    {
        List<CalendarAppointment> appointments = session.Query<CalendarAppointment>().Where(x => x.Student.Id == student.Id
                                                               && x.Start.Date >= start.Date && x.End.Date <= end.Date)
                                                               .Take(QueryLimits.Appointments).ToList();
        return appointments.ConvertToLocalTime(student);

    }

    public static List<CalendarAppointment> ConvertToUtcTime(this List<CalendarAppointment> appointments, Student student)
    {
        if (student != null)
        {
            TimeZoneInfo info = TimeZoneInfo.FindSystemTimeZoneById(student.TimeZoneId);

            foreach (var appointment in appointments)
            {
                appointment.Start = TimeZoneInfo.ConvertTimeToUtc(appointment.Start,info);
                appointment.End = TimeZoneInfo.ConvertTimeToUtc(appointment.End,info);
            }

        }

        return appointments;
    }

So I get currently the results back and then convert the times into local time. This way we don't have to worry about it.

What happens if I do this with IQueryable. Will it go off and trigger the sql anyways?

chobo2
  • 83,322
  • 195
  • 530
  • 832

1 Answers1

2

Currently I return everything from my Repo as a List. I am looking to change these to IQueryable just so people can refine the results and not suffer from another sql request(I am using nhibernate).

There are few potential problems with what you have right now and how you want to fix it. Repository should not return all objects in the first place. It encapsulates data access and provides business-driven 'collection like' interface. Repository implementation belongs to data access layer that is smart enough to not return everything:

ordersRepo.FindDelinquent();

Returning IQueryable from public method is not the solution itself, it just shifts the problem somewhere else, somewhere where it does not belong. How would you test the code that consumes this repository? What is the point of generic repository, you might as well just use NHibernate directly and couple everything to it. Please take a look at these two articles and this answer:

Timezone conversion can be moved to the CalendarAppointment itself:

DateTime end = appointement.EndTimeInStudentTimeZone(Student t)

or in

List<CalendarAppointment> appts 
         = GetAppointmentInStudentTimeZone(
                                Student student, DateTime start, DateTime end)

Or better yet convert it before you actually need to use these times (in UI or Service).

Community
  • 1
  • 1
Dmitry
  • 17,078
  • 2
  • 44
  • 70
  • I think there is a miss understanding. When I say "everything" I mean nothing all collections are returned as a list. It does not me I return every single record from the database. It will only return the records I need. I am not using a generic repo in the example I gave. Not sure what u mean by test. If I where to test it I would mock out that method and with Moq and return a result back with it. You will have to elaborate how it would look like in the entity. – chobo2 Sep 23 '11 at 20:31
  • My point is that you should not expose IQueryable as a return result from repository method. http://stackoverflow.com/questions/7513681/mocking-out-nhibernate-queryover-with-moq/7518039#7518039 – Dmitry Sep 23 '11 at 20:53
  • Hmm. What kind a method is GetAppointmentInStudentTimeZone would it be a repo method, service layer method? I originally had it so you would convert it when you needed the timezone but this led to so many problems of ppl not converting it or not know what timezone they had it in. It was decided better just have come out as in the timezone needed(as every calculation will need the right timezone anyways). If it was not for it needing a parameter in I might have made another property and call it LocalEndTime that would call the EndTime Zone and convert it it in it's get method. – chobo2 Sep 23 '11 at 21:21
  • There are multiple ways to solve your problem that have nothing to do with NHibernate. You can use one of the solutions in the answer, or use Value Objects, potentially use Noda Time as more expressive time framework etc. Feel free to ask this specific question in DDD or OOP or Language Agnostic section of the site. It really has nothing to do with NHibernate. – Dmitry Sep 23 '11 at 21:44
  • Well I still don't understand the second solution you have. I don't like the first one as it means every time you will have to call that method up also if you got them in a collection you got to loop through them. I just see ppl forgetting to do this and I will be back at square one. I am not sure what you mean by "use value objects" – chobo2 Sep 23 '11 at 21:47
  • Please open this new specific question in DDD section and I will try to answer it there. http://stackoverflow.com/questions/tagged/domain-driven-design This question is about IQueryable and repository. – Dmitry Sep 23 '11 at 21:53