@Cats-n-coffee
Posted
Hi VinÃcius!
Nice work! That's a very nice solution and handling gracefully those operations. I'll answer your question and give some feedback on what I observed (please note there are other ways to address this theme switcher):
- I would choose a radio input instead of a range input. To me it would make more sense to provide the user with a "1 choice out of 3" (which is what you did too!) but this is what radio buttons are for. It would be easier to deal with also because you can retrieve the value from the built-in element. With this, you might be able to get rid of the
switch
and directly assign the value to thebody
as a class. - You could be a little more consistent with the
onchange
event you added on the html and useaddEventListener
like you did everywhere else. - I think you can take your
addEventListener
outside each function the wrapping functions don't serve a purpose (such asexecOperations
,clear
, ...). - to maintain consistency and standards, pick one type of casing. In certain places you have a blend of camelCase and snake_case (
clearAll_button
), and in other places you use either or. In frontend the standard is camelCase. - Seems like you know how to code - not sure of your background - but depending on the task, vanilla Js (UI projects) are event driven, which mean that functions are probably tied to an event up the chain, and not often ran on their own (like your
clear
andexecOperations
do). This calculator is good project to make it only event driven with helper functions like yourcalcAdd
,calcSub
, ... . Now this is just feedback for this project, you'll find vanilla Js/Node projects that do not qualify to be mainly event driven.
Great work! Hope this helps
Marked as helpful
@Vinicius-C13
Posted
@Cats-n-coffee what a complete feedback!! It helped me a lot to see some mistakes that I didn't notice in my code. I will try to be more consistent in the standarts of my next projects and study more about event driven code. Thank you so much!