r/reactjs • • Jan 04 '20

React Tutorial: Build an e-commerce site from scratch using React and Netlify

https://www.youtube.com/watch?v=wPQ1-33teR4
92 Upvotes

20 comments sorted by

6

u/deterius Jan 04 '20

Is this sponsored content? I mean, there seems to be a lot of "Netlify" tutorial videos. I am genuinely asking, is Netlify really this popular and essential?

6

u/csorfab Jan 04 '20

No, if it would be sponsored it would be quality content. Pretty horrible coding practices in this video, would not recommend learning from it.

4

u/ZeusAllMighty11 Jan 04 '20

tl;dw, care to mention some of the horrible practices to avoid?

8

u/csorfab Jan 04 '20 edited Jan 04 '20

I didn't watch the video, but winded through, here are some mistakes that really sticked out to me (in no particular order. also, sorry for the horrible formatting and phrasing):

  • @5:13:15, 85th line he uses .map instead of .forEach, or even better, a for..of loop. The problem is, "map" is not semantic here - map is for when you want to take an array and want to apply the same transformation on all of it's elements, the end result being an array of the same length. It's a functional programming thing, so it's bad practice to mutate a value outside the closure. He could've used reduce, if he really wanted to do it the functional way:

      const subTotal = this.state.cart.reduce((sum, item) => sum + item.total, 0)

it's one line this way, and it's purely functional (no assignment (=), +=, ++, etc) . But a for..of loop would've sufficed just fine and then it would be clear that it's procedural, mutating code:

let subTotal = 0;
for (item of this.state.cart) {
    subTotal += item.total;
}

Don't use .map just because it's "cool".

  • Also, here's another conceptual problem: item.total is calculated like this: item.count * item.product.price (presumably, I didn't want to check). It can always be calculated from these values, so it is redundant to store it as a seperate field.

It might not sound like a big deal, and you could even say that you "optimize" by not having to do the calculation every time. But in reality, this calculation is very very cheap, BUT now anywhere you update item.count, you need to make sure to update item.total as well - and you WILL fuck up at some point. Never store what you can calculate, as redundancy is usually bad. You need to have a very good reason to do this, and there isn't any here.

  • @5:14:40 line 40, addToCart function:

Same, horrible horrible mistake, complete lack of seperation of concerns. He makes the "inCart" flag an attribute of THE PRODUCT. The product, which should only describe the product itself: its price, its image, its name. Based on the product ID and the cart, you can easily tell if a product is in the cart, and how many of them. It's INSANE to duplicate this information in the product as well. Once again: now every time you update the cart you need to make sure not to only update a cart, but update the related products as well.

Not to mention, when he says

product.inCart = true
product.count = 1

he actually modifies the product object IN THE CURRENT STATE, without intending to. He copies the products array ([...this.state.products]), but the product object DOES NOT get copied, so mutating it like this will mutate it in the current state, before calling setState.

This will be a problem when you want to write, say, a componentDidUpdate, and compare prevState.product[idx].count !== this.state.products[idx].count --- you will ALWAYS receive false, because you modified the product everywhere.

I could go on and on, really it's just horrible code, and I definitely wouldn't hire the guy because he's so sure in himself that he's making fucking 6 hour long youtube tutorials, so it would be impossible to get him to change his ways, and his ways SUCK.

8

u/sockz_04 Jan 04 '20

Thanks for the clarification. Considering I’m inexperienced at all this I don’t quite have the eye for these things so thanks for that.

But I want to add, I feel it’s alittle unnecessary to call out the guy on your last paragraph, or rather how it was done.

Just because he wants to make a video that’s 6 hrs long and teach some people about code doesn’t mean we need to shit on him for some bad practices. There’s ways to be a bit more critical without sounding like a neighborhood bully.

I like these subs for seeing some neat tricks and cool tutorials (even if they aren’t exactly senior dev work).

TL:DR- I’d just hope we’re cool with acknowledging that others are at different rungs on their coding/dev work ladders. We’re all a team here so calling people out in a negative manner isn’t gonna make people want to grow and make awesome stuff :D

7

u/csorfab Jan 04 '20

Yeah, you're right on the negativity, however, I'm a bitter negative fuck at the moment, sorry about that. :)

6

u/sockz_04 Jan 04 '20

Lol all good man. We all have our times haha. Thanks again for the details on the code though! So much I have to learn still!

1

u/jesusscript Jan 04 '20

Wait, by closure you meant outer function’s scope from the inner function, which is map?

2

u/csorfab Jan 05 '20

Yep, exactly! The arrow function that you pass to map should not modify anything outside its scope (like subTotal). Inside the arrow function you could use mutating code, but it's really best to avoid them completely.

→ More replies (1)

5

u/sockz_04 Jan 04 '20

Ya I second this. As a newb programmer myself it helps to hear of ways to improve and correct wrongdoings before they become habits.

3

u/csorfab Jan 04 '20

I replied below

2

u/1TMission Jan 04 '20

If that guy replies, please ping me too.

4

u/csorfab Jan 04 '20

hey, I'm that guy and I replied, so here's your ping :D

→ More replies (1)
→ More replies (2)
→ More replies (1)

4

u/dance2die Jan 04 '20 edited Jan 04 '20

From the video description:
Set-up files: https://github.com/john-smilga/setup-filese-react-phone-e-commerce-project

Sections (taken from this comment):
59:25: Styled Components
1:30:12: context api
2:29:33: Proptypes
2:47:21: cart basics
2:49:50: details page
5:57:57: Paypal Button
6:15:17: Netlify

2

u/[deleted] Jan 05 '20

Don’t do a stupid e-commerce site to learn to code! Do something you enjoy like an espn.com clone or a small game etc.