Skip to content

homework#5

Closed
spiropoulos94 wants to merge 4 commits into
SocialHackerCreteClass:masterfrom
spiropoulos94:javascript2-week1-Nikos-Spiropoulos
Closed

homework#5
spiropoulos94 wants to merge 4 commits into
SocialHackerCreteClass:masterfrom
spiropoulos94:javascript2-week1-Nikos-Spiropoulos

Conversation

@spiropoulos94
Copy link
Copy Markdown

No description provided.


for (let i=0; i<list.length; i++){
list[i].classList.add("list-item")
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It is perfectly fine as it is. Personal preference for loops is .forEach. I try to avoid for loops because you cannot understand straight away what is going on

s = checkTime(s);
document.getElementById('txt').innerHTML =
h + ":" + m + ":" + s;
var t = setInterval(showCurrentTime, 500);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

setInterval needs to be called only once. This line can live outside of this function. Also you can omit t assignment since you don't use it later on

Comment thread Week1/homework/showCurrentTime/showCurrentTime.js Outdated
</script>
</head>

<body onload="showCurrentTime()">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

like!

console.log(distance);



Copy link
Copy Markdown

Choose a reason for hiding this comment

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

no need to leave so many lines as space. One should be enough in most cases

image.style.left = `${distance}px`;
distance += 10;
console.log(image);
console.log(distance);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

finalized code should not have any console.log statements. If you want to explain something in your code, try adding some comments

@@ -0,0 +1,44 @@

let image;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This variable should not be there. If I remember correctly you placed image here so you can view it from chrome devTools console. This should not be the case. If you want to view something in the console, you can just console.log if at the right place

spiropoulos94 and others added 2 commits January 22, 2020 12:48
@spiropoulos94 spiropoulos94 deleted the javascript2-week1-Nikos-Spiropoulos branch January 23, 2020 01:02
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