green6060/digicert-public-library-demo

reading your entire file

Opened this issue · 0 comments

Honestly, I read your entire file here and still have 0 clue what this component is trying to do as a whole. Which also makes trying to give you constructive feedback nigh impossible.
Your parent div should have an identifying thing like idk... <div className={'counterIncrementer'}> or something descriptive of what it's supposed to do. The name of Counter for the component itself is very very misleading. As it's not really a counter is it? The counter component would be a component that literally just displays a number and you can increment and decrement that single number. What you have here is a single component trying to do multiple things, which should be what multiple components are for, also a better reason to use redux... since if I'm literally just displaying the counter on this exact page (or even using it a single component up in the structure) redux is way overcomplicated for what needs done. Instead we could pass a single callback function or we could do it in the same component to increments counts and such, there would be no need for redux at all. (as it stands the entirety of the "app" only uses redux to say you use redux, there's 0 reason for it that I can tell) Separate out your components, name them better. I would literally have <Counter (is this supposed to show how many books are on the page?)/> and if the counter is used to display books on the libary you'd have a state on Library (booksToDisplay) and then pass that prop down. A single callback function on 'counter' updateBooksPerPage(newNumber: number) don't use newNumber it's a bad name as well. value is also very undescriptive. and I'd bypass redux altogether and tell the people with this coding challenge to give me a real reason to use redux and then we'll add it, but that as it stands with the current project it would create more potential technical debt cause loss of efficiency and be harder to maintain in the future. Once the project grows to have a need for a global state management we can revisit then and update the locations needed. That or tell me what would be coming after this that would justify the need for redux. Long story short, don't use technologies just to use them. If I was making a hard coded static 1 page site I wouldn't even use react (except maybe for helper functions to write less code, which I have done before) Why bring a sledge hammer when you have just 1 nail to hammer in? Part of coding is using the right tools for the job.