Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion about isBetween Plugin - probably [inclusivity] as default #1223

Open
kangw3n opened this issue Nov 16, 2020 · 0 comments
Open

Discussion about isBetween Plugin - probably [inclusivity] as default #1223

kangw3n opened this issue Nov 16, 2020 · 0 comments

Comments

@kangw3n
Copy link

kangw3n commented Nov 16, 2020

I know that dayjs is based on momentjs and intent to replace momentjs seamlessly without any breaking changes on their API or methods.

There is something I would like to bring up to discuss is about the implementation of isBetween plugin.

From the documentation, we know that 4th parameter use to differentiate inclusive and exclusives between two input date. And for now the default behavior is excludes start and end date which is the string of ().

But for the most of use cases, or in term of wording choices that use to interpretation upon code reviewing, for example:

dayjs().isBetween('2020-10-01', '2020-10-30')

I would take it as to check whether TODAY fall between the start date and end date.

Due to the default behavior method which will exclude the 2 input date, which make this code abit misleading in my opinion. It might be right in term of the phrase between which should check whether a date is in between this 2 date.

But for the most cases, like the one I'm stated above, as a Developer we probably would like to set a start voting date and end voting date which we will hardcoded as a CONST , and in the end we might add the 4th params for inclusive will might be a redundant step.

const START_DATE = '2020-10-01';
const END_DATE = '2020-11-31';

function isValidVoteDate() {
  return dayjs().isBetween(START_DATE ,END_DATE);
}

app.post('/vote', (req, res) => {
  const isValid = isValidVoteDate(); // return false if today date was 2020-11-31
  if(!isValid) return;

 //...omit...//
});

I'm not sure the implement inclusive as default is a better idea for this situation, so I'm here to bring out a discussion about this problem, or perhaps adding the 4th params should be done as a best practice? Maybe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant