5
\$\begingroup\$

I have a SQLite database that only contains the holidays for 1 user.

I have a new Trip object (newTrip) that has a StartDate(long unix time) and an EndDate(long unix time) and the trip Iso3Code(string) for the location.

I need to retrieve from the database all existing Trips that can be merged with the newTrip. A trip can be merged if the dates of the new trip fall at the same time of the existing trips or on the day before or after the existing trips and have the same Iso2Code`.

So far I have this:

public static List<Trip> GetTripsThatCanBeMerged(Trip trip)
{
    //ignore times - just get for that date and get the date before for the start date and the day after for the end data, as the day before and after can be merged.
    DateTime startDate = DateTimeHelper.UnixDateToDateTime(trip.StartDate).Date.AddDays(-1);
    DateTime endDate = DateTimeHelper.UnixDateToDateTime(trip.EndDate).Date.AddDays(1);

    //now convert back to unix
    long startDateUnix = DateTimeHelper.DateTimeToUnixTimestamp(startDate);
    long endDateUnix = DateTimeHelper.DateTimeToUnixTimestamp(endDate);

    List<Trip> trips = null;

    using (SQLiteConnection conn = new SQLiteConnection(new SQLitePlatformWinRT(), _sqliteDatabasePath))
    {
        trips = conn.GetAllWithChildren<Trip>(p => 
            ((p.StartDate >= startDateUnix && p.StartDate <= endDateUnix) || 
            (p.EndDate >= startDateUnix && p.EndDate <= endDateUnix) || 
            (p.StartDate <= startDateUnix && (endDateUnix >= p.StartDate && endDateUnix <= p.EndDate))) &&
            p.Iso3Code == trip.Iso3Code
            , true);
    }

    return trips;
}

I think this is correct but can anyone suggest a less complex way of doing it, or see any errors.

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Nice first question! \$\endgroup\$ – syb0rg May 24 '16 at 16:21
  • \$\begingroup\$ Thanks. hoping someone has a better way of doing it or spots any mistakes! \$\endgroup\$ – Percy May 24 '16 at 16:22
  • 1
    \$\begingroup\$ So if (newTrip startDate) == (existingTrip endDate +1 day) the trips should be merged even if (newTrip endDate) is several days later? Or does the newTrip have to fall entirely between existingTrip (endDate +1) and (startDate - 1) ? \$\endgroup\$ – forsvarir May 24 '16 at 16:46
  • \$\begingroup\$ @forsvarir if (newTrip startDate) == (existingTrip endDate +1 day) the trips should be merged - yes to this, so e.g. existing is Mon - Wed and newTrip is Thurs - Fri, these should be merged to 1 trip of Mon-Fri \$\endgroup\$ – Percy May 25 '16 at 7:03
2
\$\begingroup\$

Let's look at this line:

(p.StartDate <= startDateUnix && (endDateUnix >= p.StartDate && endDateUnix <= p.EndDate))

If p.StartDate <= startDateUnix then p.StartDate <= endDateUnix, since startDateUnix <= endDateUnix. So endDateUnix >= p.StartDate is redundant.

So the first part of the query can be written as

(p.StartDate >= startDateUnix && p.StartDate <= endDateUnix) || 
(p.EndDate >= startDateUnix && p.EndDate <= endDateUnix) || 
(p.StartDate <= startDateUnix && endDateUnix <= p.EndDate)

I would recommend rewriting the first two conditions as follows:

(startDateUnix <= p.StartDate && p.StartDate <= endDateUnix) || 
(startDateUnix <= p.EndDate && p.EndDate <= endDateUnix)

I find it easier to see at a glance that it's saying that p.StartDate lies within [startDateUnix, endDateUnix] (similarly for p.EndDate).


Another way to test for overlapping intervals is

p.StartDate <= endDateUnix && startDateUnix <= p.EndDate
\$\endgroup\$
  • \$\begingroup\$ Based on you saying the part of the first line is redundant, I think that some of the 3rd line is redundant and can be changed to (p.StartDate <= startDateUnix && endDateUnix <= p.EndDate). Do you agree? \$\endgroup\$ – Percy May 25 '16 at 8:30
  • \$\begingroup\$ @Rick sorry if I wasn't clear, I was saying part of the third line -- not the first -- is redundant (so yes, I agree). \$\endgroup\$ – mjolka May 25 '16 at 9:20
  • \$\begingroup\$ My bad - didn't read it properly lol! \$\endgroup\$ – Percy May 25 '16 at 12:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.