Hi there, I made this project to enhance my skills in JavaScript and the Document Object Model (DOM). This is a simple project and I would appreciate your feedback about my project. Comments about my coding style will mean a lot to me. Thanks.
Link to the code - https://codepen.io/mark_akom/pen/QWGmmqV
HI @mark.akom2019 !
I think your project looks good.
Maybe the animation between colors could be a little slower.
This looks awesome! I can’t wait to the JavaScript part of the curriculum myself, lol
Your coding style is clean and I didn’t spot any severe no-gos, but I think there’s room for improvement:
Your starterColor and runColor functions do very similar things, and they also do multiple things at once. For a small project like this, that’s not a problem, but generally, you’d want to separate the logic. Looking at your starterColor function (besides the fact that it doesn’t need that color argument, you reassign color
anyway within the function), it
- generates a random colour
- updates the DOM
- keeps a log of the colours
function starterColor(p, color) {
let r = Math.floor(Math.random() * 256);
let g = Math.floor(Math.random() * 256);
let b = Math.floor(Math.random() * 256);
body.style.backgroundColor = `rgb(${r},${g},${b})`;
color = `rgb(${r},${g},${b})`;
p.textContent = p.textContent + color;
colorLog[color] = color;
}
I would refactor that into separate functions with each having a single purpose:
function starterColor(element) {
const newColor = generateColor();
updateDOM(element, newColor);
updateLog(newColor);
}
function generateColor() {
let r = Math.floor(Math.random() * 256);
let g = Math.floor(Math.random() * 256);
let b = Math.floor(Math.random() * 256);
return `rgb(${r},${g},${b})`;
}
function updateDOM(el, col) {
body.style.backgroundColor = col;
el.textContent = el.textContent + col;
}
function updateLog(col) {
colorLog[col] = col;
}
starterColor(p);
It might look like overkill to write many functions that sometimes only have one line of code, but this way, you can reuse some of them in runColor and don’t have to copy/paste the same code again in there.
Okay. I will double the current duration time
Well noted. Thanks by the way