AlekseyMonakhov / ReactNative

0 stars 0 forks source link

Feedback #12

Open mykyta-rusyn opened 9 months ago

mykyta-rusyn commented 9 months ago

I can't fully test this project, because you removed the URL from the repo.

  1. Using the wrong component. Idk why you used TouchableOpacity without the main functionality. RN has a lot of different button-like components. I think it's better to use them instead of it. <TouchableOpacity style={styles.btn} activeOpacity={1}>

  2. I recommend moving away your scrollDistance - it's the constant, why do you like to create it inside your function?

  3. Many checks. They are necessary and you can add only one if-else statement or ternary operator and return a previously created object. You can never recreate it and assign those values only once. 1 2

  4. AddToCartBtn component. You're using the same logic for different addToCart functions. Why did you not create this function in this component? And you already use favoriteStore in the AddToFavoriteBtn, so adding another logic.

AlekseyMonakhov commented 9 months ago

I can't fully test this project, because you removed the URL from the repo.

  1. Using the wrong component. Idk why you used TouchableOpacity without the main functionality. RN has a lot of different button-like components. I think it's better to use them instead of it. <TouchableOpacity style={styles.btn} activeOpacity={1}>
  2. I recommend moving away your scrollDistance - it's the constant, why do you like to create it inside your function?
  3. Many checks. They are necessary and you can add only one if-else statement or ternary operator and return a previously created object. You can never recreate it and assign those values only once. 1 2
  4. AddToCartBtn component. You're using the same logic for different addToCart functions. Why did you not create this function in this component? And you already use favoriteStore in the AddToFavoriteBtn, so adding another logic.

Very cool comments with points 2 and 3, I agree 100%, as for point number 1 - I just tested it, but I’m also not sure that this component is good. Regarding adding to the cart, I don’t want to throw a lot of props or an object into the component itself and then still put it in a function that adds or deletes. Of course, the good thing is that the items should be on the server, and only the ID that is used to add or delete should be transferred from the trash, but the world is not ideal =), so here, in order not to throw a lot of props, I just throw one function in which the required context has already been used.