IbrahiimKamal/react-redux-hooks-ecommerce

Recommended code improvments

Opened this issue · 1 comments

I would like to suggest the following code improvments:

  • Follow one file naming convention, I see that you are using 3, Capiltalized like AboutPage, Snake case like products_actions and Kabab case like about-usbanner.jpg.
  • You are directly doing http requests inside the actions. This makes the API coupled to redux. What if you some time in the future needed to fetch the products without calling a redux dispatch? It's better to separate actions from http calls.
  • It would be much more readable if you put the action type above the action itself, for example:
export const FETCH_PRODUCTS = 'FETCH_PRODUCTS'
export function fetchProducts = () => {
  return async () => ...
}
  • Some components are directly depending on redux. For example AllProducts gets the loading state from redux store directly. What if you wanted to replace redux with something else in the future ? you will have to visit all these components and change them. Instead, get the loading state from props and only connect the page itself (the container) and not the component (the representational one).
  • You have some repeated code like
<section className="py-5">
        <div className="container">
          <Title title="BEST SELLING" />
          <div className="row">

once for loading and once to show products. Why not using the conditional rendering ?

  • You have some static css (not going to chage) but being passed as a style object to the component like
<div
          style={{ textAlign: 'center' }}
          className="col-10 mx-auto pt-3"
>

This is going to re-render the component in every update although it might not need to be re-rendered.

  • I suggest doing some components for smaller elements like the button and the input so that you don't have to write css classes again again.
  • There are some useless comments like this one
// import navbar links from utils
import { navbarLinks } from '../../utils/navbarLinks';
  • A component is either representational or connected, mixing both in one component is not good.

However, the code overall is actually good and you are great at styling. best of luck :D and sorry for annoying

thanks so much