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

add find date feature #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add find date feature #2

wants to merge 1 commit into from

Conversation

parasmehta
Copy link
Owner

No description provided.

let ret = null;
let matches = [];
for (let i = 0; i < fragments.length; i += 1) {
const m1 = fragments[i].match(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Checks m1 and m2 will result in adding wrong dates to matches array. For dd.mm.yyyy format, m2 will push wrong date in matches (eg "01.01.2012 will be pushed as "01.01.20"). To avoid this, checks for date formats dd.mm.yyyy and dd.mm.yy can be combined into one.
Suggested change
const m1 = fragments[i].match(
const m1 = fragments[i].match(/[0-9][0-9]\.[0-9][0-9]\.(\d{2}|\d{4})+/g);
  1. dates can already be converted to date objects using new Date() before pushing to matches. In this way, conversion in multiple lines (47-51) can be avoided. Also, it will be useful in further validation of input dates.

  2. Pseudo code for the flow:

  • Validate date format
  • If valid, convert to date object and push to matches
  • Remove invalid dates from matches array
  • Check for closest day from today. Add a check to ignore future dates and dates older than 10 years
  • Return JS date object or null (if no strings contain a date)

if (parts[2].length === 2) {
y = `20${y}`;
}
//d.setFullYear(y);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove all unused/commented code

y = `20${y}`;
}
//d.setFullYear(y);
//d.setMonth(parseInt(parts[1], 10));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove all unused/commented code

if (matches.length > 0) {
const dates = [];
for (let i = 0; i < matches.length; i += 1) {
if (matches[i]) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 33- 44. These conversions can be done before moving input dates to matches.

}
//d.setFullYear(y);
//d.setMonth(parseInt(parts[1], 10));
d.setFullYear(parseInt(y, 10));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LInes 47-51, can be removed as we create all input dates to date objects in the beginning

for (let i = 0; i < dates.length; i += 1) {
if (!isNaN(dates[i].getTime())) {
const td = Date.now() - dates[i].getTime();
if (td < datediff) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Future dates should be ignored. td for future dates will always return -ve values. Please add a check to ignore future dates while calculating date closest to today.

Suggested change
if (td < datediff) {
if (Math.sign(td) !== -1 && td < datediff)

}
}
}
ret = dates[ind];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be set inside the if (Math.sign(td) !== -1 && td < datediff) block. Otherwise, it will always be set to dates[0] and return wrong result.

});

it("finds yyyy-mm-dd", () => {
const fragments = ["asd", "12.03.15", "34983"];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const fragments = ["asd", "12.03.15", "34983"];
const fragments = ["asd", "2015-03-12", "34983"];

matches = matches.concat(m3);
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, please add a function to check if all the input dates are valid. Any invalid date should not be passed further.
Pseudo code:
let newMatches = isValid(matches)

function isValid(matches) {
This function should return only valid dates
}

Use newMatches for further validation

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

Successfully merging this pull request may close these issues.

2 participants