Skip to content

Update main.js#1

Open
AlionaTytarenko wants to merge 17 commits into
Kirill380:masterfrom
AlionaTytarenko:patch-1
Open

Update main.js#1
AlionaTytarenko wants to merge 17 commits into
Kirill380:masterfrom
AlionaTytarenko:patch-1

Conversation

@AlionaTytarenko
Copy link
Copy Markdown

No description provided.

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

if (dayOfWeek == 1) {
Copy link
Copy Markdown
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

Comment thread date/main.js Outdated

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

if ([01, 10].includes(month)) {
Copy link
Copy Markdown
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

Comment thread 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
Copy Markdown
Owner

Choose a reason for hiding this comment

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

just !date

Comment thread 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
Copy Markdown
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()

Comment thread 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
Copy Markdown
Owner

Choose a reason for hiding this comment

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

see comment above

Comment thread 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
Copy Markdown
Owner

Choose a reason for hiding this comment

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

see comment above

Comment thread date/main.js Outdated
alert (yyyy);
};
yyyy++
} while (yyyy !== 2002); No newline at end of file
Copy link
Copy Markdown
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
Copy Markdown
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

Comment thread date/test.js Outdated
}

for (let i = 0;
i < dateInvalidValue.length; i++) {
Copy link
Copy Markdown
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
Copy Markdown
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 ;)

Comment thread date/test.js Outdated
});
}

for (let i = 0;
Copy link
Copy Markdown
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

Comment thread date/test.js Outdated
}


let dayInvalidValue = ['12,2.01.1234', '32.2.00', ' ', '0.-.='];
Copy link
Copy Markdown
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
Copy Markdown
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(

Comment thread date/main.js Outdated

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

yyyy = 1997;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

let is missing

Comment thread date/main.js Outdated

yyyy = 1997;
do {
dayOfWeek(getDate(13,07,yyyy));
Copy link
Copy Markdown
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