Check if calendar week number falls within two other week numbers

Question

I want to check with Javascript if a week number (with year) is within two other week numbers.

Example data: Year-Week startDate: 2020-32 endDate: 2021-52 checkDate: 2021-30. In this case checkDate is within the two years.

How do I make Javascript know this too? I figured I should check if the date is bigger than startDate and smaller than endDate. But doing these checks on both years and weeks seems to get quite big. Here is what I have implemented:

/**
 * Returns whether the week is within the two given weeks or not.
 * 
 * @param {Number} startYear Example data: 2020
 * @param {Number} startWeek 32
 * @param {Number} endYear 2021
 * @param {Number} endWeek 52
 * @param {Number} checkYear 2021
 * @param {Number} checkWeek 30
 * @returns true or false
 */
function isWeekWithinTwoWeeks(startYear, startWeek, endYear, endWeek, checkYear, checkWeek) {
    // If checkYear is same as startyear and checkWeek bigger or equal to startWeek and checkWeek smaller or equal than endWeek, its not allowed.
    // Else if checkYear is bigger than startYear, and checkYear is smaller than endYear it means it is in between.
    // Also if checkYear is bigger than startYear but checkYear is not smaller but equal to endYear, and checkWeek is smaller or equal to endWeek, it is in between.
    if (checkYear === startYear) {
        if (checkWeek >= startWeek) {
            if (checkWeek <= endWeek) {
                addUserFormError.innerHTML = "Not allowed to have a week within two other weeks!";
                return false;
            }
        }
    } else if (checkYear > startYear) {
        if (checkYear < endYear) {
            addUserFormError.innerHTML = "Not allowed to have a week within two other weeks!";
            return false;
        } else if (checkYear === endYear) {
            if (checkWeek <= endWeek) {
                addUserFormError.innerHTML = "Not allowed to have a week within two other weeks!";
                return false;
            }
        }
    }

    return true;
}

I think it could be done in a much simpler way and would like to know if there is much to improve here and how. I do know I could use && signs in the if statements to make them more of a one-liner, but thought this might be more readable.

Answer 1

Logic

Your logic is not entirely correct, consider the following arguments:

startYear: 2001
startWeek: 10
endYear: 2002
endWeek: 5
checkYear: 2001
checkWeek: 15

checkYear === startYear is true, but checkWeek <= endWeek is not, leading to true output, which is not intended here. The check for checkWeek <= endWeek is only relevant if startYear === endYear.

---

Nested ifs vs. &&

This point does not consider the aformentioned logic issues, simply focuses on code style

I'd argue using the &&-operator here actually increases readability. You should consider that each level of indentation is another layer of context the reader needs to keep track of in their head. We're already two levels of indentation deep once we get to this check, which then (unnecessarily) adds another two levels of indentation.

if (checkWeek >= startWeek) {
    if (checkWeek <= endWeek) {
        // Do something
    }
}

Using the &&-operator here is also closer to the natural thought process: If the week number is greater than or equal to startWeek AND less than or equal to the endWeek it is between the two. I'd say this is closer to a single logical unit instead of two subsequent logical units (as your implementation suggests).

if (checkWeek >= startWeek && checkWeek <= endWeek) {
    // Do something
}

Please note that there is no absolute truth here, personal preference, use case and code style guidelines are always relevant factors.

---

Function naming and return values

You have your function naming / return values the wrong way round. If I call isWeekWithinTwoWeeks(...) I expect to get true if my week number lies between start and end. Your function returns false for this case. Either your function name or your return values should be the exact opposite. This is extremely relevant for readability. As it is now, I would need to inspect and understand the implementation of every single function to be sure what they do. Intuitive naming enables readers to understand your code more easily and reliably.

---

Side effects

I don't know the structure of your project, but you might (/ should) consider removing this side effect

addUserFormError.innerHTML = "Not allowed to have a week within two other weeks!"

from this particular function. I would not expect a function called isWeekWithinTwoWeeks to directly modify the DOM. Instead I would move it to wherever the function is called from, dependent on the return value of isWeekWithinTwoWeeks. This reduces code duplication (the aforementioned line is repeated three times) and further increases readability as functionality is more closely related to naming.

Answer 2

DOM access

The review by riskypenguin already covered side-effects. Additionally, addUserFormError doesn't appear to be defined in the Javascript. Maybe this is because that wasn't included with the JS code snippet but if it is just access it via the global DOM then consider using a DOM access method instead to define it - e.g.

const addUserFormError = document.getElementById('addUserFormError);

Depending on the global DOM is not recommended as global variables can easily be overwritten.

Answer 3

Managing dates is difficult. Don't write your own complex functions with complicated logic which will inevitably either fail at some edge case or cause confusion when trying to read it.

Instead, use a well-tested library to handle this for you. With Luxon, you can parse the year and week number to create a date time object. You can then compare these objects directly. It's extremely simple and readable.

import { DateTime } from "luxon";

const startDate = DateTime.fromISO(`${startYear}-W${startWeek}-1`);
const endDate = DateTime.fromISO(`${endYear}-W${endWeek}-1`);
const checkDate = DateTime.fromISO(`${checkYear}-W${checkWeek}-1`);

return checkDate > startDate && checkDate < endDate;

Answer 4

Its been some time but I came with a different improvement. One that seems quite simple to me.

/**
 * Returns whether the week is within the two given weeks or not.
 * 
 * @param {Number} startYear
 * @param {Number} startWeek
 * @param {Number} endYear
 * @param {Number} endWeek
 * @param {Number} checkYear
 * @param {Number} checkWeek
 * @returns true or false
 */
function isWeekWithinTwoWeeks(startYear, startWeek, endYear, endWeek, checkYear, checkWeek) {

    // Calculations would result in something like: 202101. Which is year 2021 and week 01.
    startDate = startYear * 100 + startWeek;
    endDate = endYear * 100 + endWeek;
    checkDate = checkYear * 100 + checkWeek;

    // Check if check date is between start and end date, or end and start date. 
    // Credits to TobySpeight for the improvement of this part. See my edits for old version.
    return checkDate >= startDate && checkDate <= endDate || checkDate >= endDate && checkDate <= startDate;
}

Added an extra if to make sure true will be returned when start year/week is bigger than end year/week (which should not make sense but could happen) and check year/week is still in between.

The calculations made makes it much more simpler to check if a week is in between or not.