Skip to content

Conversation

@AlionaTytarenko
Copy link

No description provided.

date/main.js Outdated
sumShift = (centuryShift + yearShift + monthShift + dayShift);
dayOfWeek = Math.round(sumShift/7);

if (dayOfWeek == 1) {
Copy link
Owner

Choose a reason for hiding this comment

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

use array for getting the name of day of week by index

date/main.js Outdated

yearShift = ((decimalYear + (decimalYear/4)) % 7);

if ([01, 10].includes(month)) {
Copy link
Owner

Choose a reason for hiding this comment

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

use array for getting the shift of month by index

date/main.js Outdated
// https://artofmemory.com/blog/how-to-calculate-the-day-of-the-week-4203.html
// https://wpcalc.com/kak-uznat-den-nedeli-lyuboj-daty/

if (!date || date.length == 0) {
Copy link
Owner

Choose a reason for hiding this comment

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

just !date

date/main.js Outdated
let monthNumber = Number(arr[1]);
let yearNumber = Number(arr[2]);

if (Number(dayNumber) != Number(dayNumber) || dayNumber != dayNumber || dayNumber < 0 || dayNumber > 31 || dayNumber == null || dayNumber == Infinity || dayNumber == -Infinity || dayNumber == 0) {
Copy link
Owner

Choose a reason for hiding this comment

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

move checks only related to number to a separate function, give it a name isNumber()

date/main.js Outdated
if (Number(dayNumber) != Number(dayNumber) || dayNumber != dayNumber || dayNumber < 0 || dayNumber > 31 || dayNumber == null || dayNumber == Infinity || dayNumber == -Infinity || dayNumber == 0) {
throw new Error('Invalid value of day')
};
if (Number(monthNumber) != Number(monthNumber) || monthNumber != monthNumber || monthNumber < 0 || monthNumber > 12 || monthNumber == null || monthNumber == Infinity || monthNumber == -Infinity || monthNumber == 0) {
Copy link
Owner

Choose a reason for hiding this comment

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

see comment above

date/main.js Outdated
if (Number(monthNumber) != Number(monthNumber) || monthNumber != monthNumber || monthNumber < 0 || monthNumber > 12 || monthNumber == null || monthNumber == Infinity || monthNumber == -Infinity || monthNumber == 0) {
throw new Error('Invalid value of month')
};
if (Number(yearNumber) != Number(yearNumber) || yearNumber != yearNumber || yearNumber < 0 || yearNumber == null || yearNumber == Infinity || yearNumber == -Infinity || yearNumber == 0) {
Copy link
Owner

Choose a reason for hiding this comment

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

see comment above

date/main.js Outdated
alert (yyyy);
};
yyyy++
} while (yyyy !== 2002);
Copy link
Owner

Choose a reason for hiding this comment

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

1997 + 120 = 2117 not 2002

Copy link
Author

Choose a reason for hiding this comment

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

oops, changed it just for fast debugging

date/test.js Outdated
}

for (let i = 0;
i < dateInvalidValue.length; i++) {
Copy link
Owner

Choose a reason for hiding this comment

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

should be on the same line with initialization of i (let i = 0), i is code style convention

Copy link
Author

Choose a reason for hiding this comment

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

... remember you were trying to beautify markup? it wasnt me who was trying ;)

date/test.js Outdated
});
}

for (let i = 0;
Copy link
Owner

@Kirill380 Kirill380 Dec 14, 2019

Choose a reason for hiding this comment

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

you don't need a loop for only one value

date/test.js Outdated
}


let dayInvalidValue = ['12,2.01.1234', '32.2.00', ' ', '0.-.='];
Copy link
Owner

Choose a reason for hiding this comment

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

I read the mocha documentation and found a more easier and shorter way to write parameterized tests (actualy I can figure out by my own):

['12,2.01.1234', '32.2.00', '     ', '0.-.='].forEach(x => {
       it(`${x} throws exception`, function() {
                assert.throws(() => dayOfWeek(x), 'Invalid value of day');
        });
})

Copy link
Author

Choose a reason for hiding this comment

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

Im not sure I did this properly, tests broke down after my fix(

date/main.js Outdated

function getDate(dd,mm,yyyy) {return `${dd}.${mm}.${yyyy}`}

yyyy = 1997;
Copy link
Owner

Choose a reason for hiding this comment

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

let is missing

date/main.js Outdated

yyyy = 1997;
do {
dayOfWeek(getDate(13,07,yyyy));
Copy link
Owner

Choose a reason for hiding this comment

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

zero before 7 is redundant

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