How can I structure this code better?

Question

I really want to know how much more structure you can do for a task like this in a class cause right now I'm in shock that my answer wasn't enough for this company's interview question. They literally asked me to take a date and see if it is between today and 5 days prior. So let me know how much more needs to be done here.

The date in question is passed to a function. That function takes today's date and then you compare the date in question from 5 days prior to today's date. If the date in question is ascending from the 5-days-prior and then if the date is descending from today's date the function returns true. So someone let me know where I went wrong here.

func compareDate(thisDate:Date,aheadOfDate:Date, beforeThisDate:Date) -> Bool{
    if( thisDate < aheadOfDate){
        return false
    }
    if(thisDate > beforeThisDate){
        return false
    }
    return true
}

func daysFromToday(days:Int) -> Date{

    let calendar = Calendar.current
    let newDate = calendar.date(byAdding: .day, value: days, to:  Date.init())
    return newDate!
}

let todaysDate = Date.init()
let fiveDaysAgo = daysFromToday(days:-5)

print(compareDate(thisDate: daysFromToday(days: 1), aheadOfDate: fiveDaysAgo, beforeThisDate: todaysDate))

Answer 1

It would be simpler to subtract the given date from today's date and see if the difference in days is between 0 and 5. Also, IDK if they care, but the compareDate function could instead be a one-line boolean interpretation.

Answer 2

First function

I would restructure into an extension on Date:

extension Date {
    func isBetween(_ first: Date, and second: Date) -> Bool {
        return first < self && self < second
    }
}

Then I would replace this a < b && b < c pattern with a range expression a...c ~= b, which creates a date range, and checks if self is in it:

extension Date {
    func isBetween(_ first: Date, and second: Date) -> Bool {
        return first...second ~= self
    }
}

Second function

I would also restructure into an extension on Date, and I would inline calendar and newDate because they add visual clutter, without providing any new information. I would also replace Date.init() with just Date().

extension Date {
    func offset(days: Int) -> Date {
        return Calendar.current.date(byAdding: .day, value: days, to:  self)
    }
}

Usage

In the final snippet, I would use these functions as such:

let now = Date()
let fiveDaysAgo = now.offset(days: -5)

print(now.offset(days: +1).isBetween(fiveDaysAgo, and: now))

Answer 3

I think what the current answers are missing is that if you want to compare dates you need to take the time out of the equation (which is not the same as not caring about the time). So for that interview question what you are really saying is 'is the date I am checking >= the start of the day 5 days ago and before the start of tomorrow'. After all if you are checking it at 10pm today then 9am 5 days ago would still be ok but not if you are including time in all the checks.

So before doing any checking you need to calculate the start of day for 5 days ago and tomorrow which is done something like this:

    let now = Date()

    let cal = Calendar.current
    var start = cal.startOfDay(for: Date())
    let inYesterday = cal.date(byAdding: .day, value: -5, to: now)!
    start = cal.startOfDay(for: inYesterday)

    var end = cal.startOfDay(for: Date())
    let inTomorrow = cal.date(byAdding: .day, value: 1, to: now)!
    end = cal.startOfDay(for: inTomorrow)

(This is more complex than you would think because you have to account for different time zones and formats and things like summer time.)

Then depending on how you want to use it you could do something like this:

extension Date {
    func isBetween(date: Date, andDaysAgo daysAgo: Int) -> Bool {
        let cal = Calendar.current
        var start = cal.startOfDay(for: date)
        let inYesterday = cal.date(byAdding: .day, value: -daysAgo, to: date)!
        start = cal.startOfDay(for: inYesterday)

        var end = cal.startOfDay(for: date)
        let inTomorrow = cal.date(byAdding: .day, value: 1, to: date)!
        end = cal.startOfDay(for: inTomorrow)

        return start..<end ~= self
    }
}

Which you would call like this:

var checkDate = Date(timeIntervalSinceNow: 3600 * 24 * 0)
print (checkDate.isBetween(date: Date(), andDaysAgo: 5) // prints true
checkDate = Date(timeIntervalSinceNow: 3600 * 24 * -5)
print (checkDate.isBetween(date: Date(), andDaysAgo: 5) // prints true
checkDate = Date(timeIntervalSinceNow: 3600 * 24 * 1)
print (checkDate.isBetween(date: Date(), andDaysAgo: 5) // prints false
checkDate = Date(timeIntervalSinceNow: 3600 * 24 * -6)
print (checkDate.isBetween(date: Date(), andDaysAgo: 5) // prints flase

(these are just quick hacked examples)

EDIT

As was pointed out to me (quite rightly) if you are comparing dates and don't care about time then you can use noon as your reference point (basically it's always fixed and unaffected by things like daylight saving changes). So this is one way to do that:

func isBetweenAlt(date: Date, andDaysAgo daysAgo: Int) -> Bool {
    let cal = Calendar.current
    let startCheck = cal.date(bySettingHour: 12, minute: 0, second: 0, of: cal.date(byAdding: .day, value: -daysAgo, to: date)!)!
    let endCheck = cal.date(bySettingHour: 12, minute: 0, second: 0, of: date)!
    let checkDate = cal.date(bySettingHour: 12, minute: 0, second: 0, of: self)!

    return startCheck...endCheck ~= checkDate
}

Also in the spirit of learning here are a couple of other ways to do it:

func isBetweenAlt2(date: Date, andDaysAgo daysAgo: Int) -> Bool {
    let cal = Calendar.current
    let startCheck = cal.date(byAdding: .day, value: -(daysAgo + 1), to: date)! // You have to offset by one day as the test will be > not >=
    let endCheck = cal.date(byAdding: .day, value: 1, to: date)! // You have to offset by one day as the test will be < not <=

    return cal.compare(startCheck, to: self, toGranularity: .day) == .orderedAscending && cal.compare(endCheck, to: self, toGranularity: .day) == .orderedDescending
}

func isBetweenAlt3(date: Date, andDaysAgo daysAgo: Int) -> Bool {
    let cal = Calendar.current
    let startCheck = cal.ordinality(of: .day, in: .era, for: cal.date(byAdding: .day, value: -daysAgo, to: date)!)!
    let endCheck = cal.ordinality(of: .day, in: .era, for: date)!
    let check = cal.ordinality(of: .day, in: .era, for: self)!

    return startCheck...endCheck ~= check
}

All of them do the same job (I think).

EDIT

With reference back to the original question and these kind of tests in interviews. Often there are multiple solutions to a problem that may all be equally valid and the idea of the interview question is not necessarily to get the answer they think is correct but to show that you have thought about the issues that affect the problem. Particularly with a junior developer role it can be less important that an answer is complete in the time but more important that the person understood the issue, the challenges involved and how to go about solving it.

Answer 4

I made your code a bit shorter and a bit more "swift-like"

func compare(date thisDate: Date, aheadOf aheadOfDate: Date, before beforeDate: Date) -> Bool {
    return (thisDate > aheadOfDate) && (thisDate < beforeDate)
}

func fromToday(days: Double) -> Date {
    let today = Date()
    return Date(timeIntervalSince1970: today.timeIntervalSince1970 + (days * 86400.0)) //Todays date in seconds + seconds in a day times desired number of days
}

let today = Date()
let fiveDaysAgo = fromToday(days: -5)
let oneDayFromToday = fromToday(days: 1)

print(compare(date: oneDayFromToday, aheadOf: fiveDaysAgo, before: today))