Skip to content

#1-4Task. Anton Demidovich - #781

Draft
AnthonyDem wants to merge 6 commits into
ilya4321:masterfrom
AnthonyDem:master
Draft

#1-4Task. Anton Demidovich#781
AnthonyDem wants to merge 6 commits into
ilya4321:masterfrom
AnthonyDem:master

Conversation

@AnthonyDem

Copy link
Copy Markdown

End my tasks 1-4

@AnthonyDem AnthonyDem closed this Jul 25, 2020
@AnthonyDem AnthonyDem reopened this Jul 25, 2020

@vramaniuk vramaniuk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please, try to follow these rules https://javascript.info/coding-style

I'll mark the PR as "Draft", please click "ready for review" when it will be finished. Thank you!

Please, privide the direct link to your task success travis build

Comment thread task/01-strings-tasks.js
*/
function getStringFromTemplate(firstName, lastName) {
throw new Error('Not implemented');
return 'Hello, ' + firstName + ' ' + lastName + '!'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Comment thread task/01-strings-tasks.js
*/
function unbracketTag(str) {
throw new Error('Not implemented');
return str.replace('<', '').replace('>', '')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please, use just 1 replace with regexp

Comment thread task/01-strings-tasks.js
Comment on lines +216 to +229
for (var h=0; h<height; h++) {

if (h===0){
shape += '┌' + '─'.repeat(width-2) + '┐\n';
}else if (h===height-1){
shape += '└' + '─'.repeat(width-2) + '┘\n';
}
else{
shape += '│' + ' '.repeat(width-2) + '│\n';
}

}
return shape;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please, try to follow these rules https://javascript.info/coding-style

HERE and below (spaces around operators)

Comment thread task/01-strings-tasks.js
Comment on lines +296 to +301
if (typeof value === 'string' || value instanceof String)
{
return true;
}else {
return false;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

just return (typeof value === 'string' || value instanceof String)

Please, see https://javascript.info/logical-operators

Comment thread task/02-numbers-tasks.js
*/
function getDistanceBetweenPoints(x1, y1, x2, y2) {
throw new Error('Not implemented');
return Math.sqrt(Math.pow((x1-x2),2) + Math.pow((y1-y2),2));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please, improve using Math.hypot

Comment thread task/04-arrays-tasks.js
function toCsvText(arr) {
throw new Error('Not implemented');
function toCsvText(arr) {
return arr.map(elem=>elem.join(',')).join('\n');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We don't need any loops here.
Try to solve ising just join

Comment thread task/04-arrays-tasks.js
*/
function getFalsyValuesCount(arr) {
throw new Error('Not implemented');
return arr.filter(elem=>Boolean(elem)===false).length;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

return arr.filter(elem=>!elem).length; will work

Please, see https://javascript.info/logical-operators

Comment thread task/04-arrays-tasks.js
throw new Error('Not implemented');
let dict = new Map();
array.map(each_el => {
let arr = new Array();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

let arr = []; preferable way to initialize empty array

Comment thread task/04-arrays-tasks.js
let dict = new Map();
array.map(each_el => {
let arr = new Array();
if (dict.get(keySelector(each_el)) === undefined) dict.set(keySelector(each_el), new Array(valueSelector(each_el)));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if (dict.get(!keySelector(each_el))) dict.set(keySelector(each_el), new Array(valueSelector(each_el))); - preferable

Comment thread task/04-arrays-tasks.js
let new_arr = new Array();
let arr_index = 0;
indexes.map(each_el => {
if (each_el != 0) arr_index = each_el;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

"operator !== is always preferable
sometimes ''!=' may cause an error
(check at console :
'0' != 0
and '0' !== 0)"

@vramaniuk
vramaniuk marked this pull request as draft August 27, 2020 18:12
@AnthonyDem
AnthonyDem requested a review from vramaniuk September 2, 2020 17:01
@AnthonyDem
AnthonyDem marked this pull request as ready for review September 2, 2020 17:03
@vramaniuk

Copy link
Copy Markdown
Collaborator

Why did you marked this PR as ready for review?
You should implement all notes from above

@vramaniuk

Copy link
Copy Markdown
Collaborator

I'll mark the PR as "Draft", please click "ready for review" when it will be finished. Thank you!

@vramaniuk
vramaniuk marked this pull request as draft September 2, 2020 17:18
@vramaniuk

Copy link
Copy Markdown
Collaborator

And , please, resolve all conflicting files (readme.md, package....)

@AnthonyDem

Copy link
Copy Markdown
Author

Why did you marked this PR as ready for review?
You should implement all notes from above

I was think i did it ? In my repository all tasks are was fixed

@vramaniuk

Copy link
Copy Markdown
Collaborator

You have no specific branch for task 1-4. It should be called like 1_4_task or something like that.
Selection_003

@vramaniuk

Copy link
Copy Markdown
Collaborator

For tasks1-4, task5, task6 ... you should create new branch from master branch and then make the PR

@AnthonyDem

Copy link
Copy Markdown
Author

You have no specific branch for task 1-4. It should be called like 1_4_task or something like that.
Selection_003

Okay, thank you , i will do it )

@vramaniuk

Copy link
Copy Markdown
Collaborator

Please, be carefull with all the notes
I have found , that not all notes are implemented even in master branch

Selection_004

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