Refactoring A Junior’s React Code - 43% Less Code With A Better Data Structure
Вставка
- Опубліковано 6 чер 2024
- Find the blog post to this video including the complete code here:
profy.dev/article/react-junio...
Find a detailed guide to debugging React with VS Code here: profy.dev/article/debug-react...
00:00 - Intro
00:08 - Demo of the app
00:35 - Code Review: Overview
01:28 - Code Review: Data Structure
02:18 - Code Review: Unnecessary State
02:43 - Code Review: Directly Accessing The DOM
03:03 - Refactor: Data Structure
07:26 - Refactor: Unnecessary State
08:52 - Refactor: Remove document.getElementById()
10:29 - Refactor: Final Clean Up
11:36 - Final Review & Summary
100% do more refactoring videos, these are rare and idk if it's specifically your unique idea to do this, but this is actually gold content
This is great, would love to see more of these.
Great stuff! I've definitely been sleeping on the usefulness of maps! Subscribed :)
I'm a junior dev, and I was wondering why you choose map as the state data structure instead of a set, because you're not keeping the state checked per id, you're only keeping the ones that are checked. And even if you kept all ids and just toggled between true or false, in this context it still makes more sense to me to use a set.
Yes you’re right. Iirc I changed the code to use a set after recording this video. You can check the blog post for that implementation. That’s the advantage of a blog post: you can’t just adjust the video but the blog is easy 🙂
Mapping is unnecessary all together and you can manage the entirety of this component using a single state. Anyone else also hate the fact that they don’t separate the styling from the implementation of the functional component? You can style elements based on the state of an input in css..
Great video Johannes! I had your blog on this lined up for reading for a year now. I saw the video and just went for it now. 😆
Very good!
Yes, I want to see more of this stuff.
The refactoring is just on point. Good work.
This is was amazing, would love to see more content like this
Great work brother . Expecting more like this 👍👍
Keep doing these awesome videos. I find them so helpful!
This is excellent! getting back into React after a long break, and your videos are a godsent. Keep it up!
Thanks :)
Not gonna lie, this is extremely valuable information for junior developers (and even more experienced ones), thank you for sharing your knowledge, and by the way, your explanations are very clear
Nice! This was a good example of simplifying the code. I think it would be interesting to see how you might word a review on an example PR like this one.
I wish I saw this YEARS ago, that was awesome and insightful
Awaiting more video like this, Thanks for posting!!
Awesome. Would like to see more videos like this
I love these kinds of videos! Great stuff!
Great video! I would love to see more of these
This Is really helpful as a junior dev myself!
Absolutely loved this article. I read it last night and could apply it in some work I went back to today, where I'd added unnecessary state that I could've derived
Thanks so much for the feedback!
Awesome stuff, would definitely love to see more of this. Liked 'n subscribed!
Nicely done!
As a senior developer I can assure that your work here is a gem.
Thanks, that means a lot
WE NEED MORE OF THIS CONTENT
Great video, really helpful for someone getting started!
Wow this video was actually amazing, that's wild!
Subscribed! I want more of this. 🤩
This is wonderful, this kind of content is very helpful
Thanks. As a just learned react developer, this will help me
thanks for this, helps a lot as a junior
Thank you for this video! Helpful 💓💓
Really cool video! Thanks!
A thought on the data structure you chose. A map works well. Whenever I see a map where the value is a Boolean, I ask myself if a Set would work instead. In this case, I think what you really want is a Set.
Never mind I see you addressed this in another comment. I didn’t see that until now! Thanks
Loved this kind of video!
I subbed, hoping to see more refactoring videos!!
Hi, hope you will have more of this video series, or refactoring playlist. Thanks for the content! thumbs up!!
Thanks for the feedback. Check my React Architecture playlist. That's basically a larger refactoring as well only with a focus on architecture
aah I could watch these all day. Loving it! Very tempted to send my code... hoy could I do that? Cheers!
Excellent refactor, well done
Great video :)
Thanks for sharing. Cool work.
One thing I could suggest is to split component into several smaller once.
That's a good suggestion. Honestly I didn't think that much about the JSX. But yeah, especially the table row would be a good candidate for a separate component imo
Very informative!
I would love to see more
Thank you so much. If possible, please upload a video on how to develop a production ready react app, for example the folder structure, custom hooks, global context store, global axios confg, etc.
more videos like this please!
thank you very much for the video
Last issue is not using typescript
Juniors code is better than some of the seniors where I work.
very nice, clear and helpful for me as im practicing with react! although i needed to watch it in 0.75 but thats because im not native english 😅
Amazing video ! May I ask when is it appropriate to use a map vs an array?
Do it just do it this is awesome
Boss level♥
wow, great work! how long would it take you to do these refators in real time? I imagine its not done typically within 20 minutes... thanks for the video
I gotta say.. your tone and presentation is super monotonous,
but the value of what you say and explain is great. Well done
I agree with all you said. One thing about the Map though. Personally I dislike using Maps on very big data arrays,
and simply revert to arrays + writing helper functions for search and filtering but a Map is much more readable and "easier" to use
Nice! Would be interesting to see how the solution can be extended for a filterable list
Hello, as your map of completed issues only ever has a value of true, was the only reason you used a map instead of an array/set here so the lookup could be faster when checking if the issue is completed or not?
A set of IDs seems to make more sense imo
Thanks
AFAIK looking up in a map has a constant access time due to it being stored as a hashtable behind the scenes. Meaning it will be the same access time no matte how many elements will be in that list.
@@MichaelKireYou are right, but so do sets. Essentially a set is a map mapping to booleans. But I'm not a javascript guy so I also don't know why a map was preferred over a set here.
Scrolled a little more, turns out there is no good reason and he ended up changing to a map and updating the blog post but didn't redo the video (wouldn't expect him to)
good video! I would make a habit of saying "simply" less often. what's simple and obvious to you might not be to others!
Unnecessary state variables are the worst.
As a beginner it's easier to make the mistake... Not a lot of tutorial drive home the concept of derived state well. So as a a beginner you tend to default to patterns you are aware of.
This was very insightful. As a junior dev I learned a lot. Most importantly is that I really have to start using the new Map instead of just objects and I actually didn't know about the ".size" property. does it work with "regular" objects?
Last question is a bit longer but since you touched on that subject I'll allow myself to ask that:
I have an app in which I have a sidebar with buttons which run functions (ofc) and one of those uses the text in the main layout. In that case, using a ref felt like bad practice since I would have to either forward it through many cmps, store it in the global store or use context instead of just accessing it directly... what is your take on that? Thanks! :)
Thanks a lot for the feedback. You only have the size on Map and Set not on regular objects.
Regarding the shared text: not sure but this might be a good case for using context. Maybe there's also an alternative way of structuring the app that's more efficient. Hard to say. If you want me to do a similar review feel free to send me the code at review@profy.dev
@@profydev Thanks for the comment (I subbed by the way, keep it up, I know many people are looking for content exactly like that).
I wish I could, that would be very useful. but it's quite a big app which I wrote for work and it's on a closed network :( Can I email you the question with more context anyway?
@@Tom_Rose Sure, feel free to send me an email to johannes@profy.dev. There's no need to share the whole code btw. A reduced version on CodeSandbox would even be better
One question , the thing where you replace the for loop for counting elements that have the status open with a filtered array ( arr.filter().length ) just to have it just "pretty" impact the memory usage ? U don't just iterate thought the the array that you have , u create a new array so , allocating more memory .Maybe a middle ground can be a reducer :D
Nvm , I didn't get to the part where you mentioned about memory :D , great job XD
Haha thanks
a note on Map and Set Javascript structures. with Node.js it's fine to use them, but avoid using them on v8 (browser). V8 uses a ton of optimization techniques, specifically for basic array and objects that cannot be done for Map and Set's. to be even more specific, Map and Set cannot be cached as effectively and thus become slower than if it was otherwise implemented with a basic object or array. I'm not 100% sure if this is still the case but it was when this video came out.
Thanks for the hint
Doesn't Node.js also use V8?
Omg u do it all so fast compared to me, it makes me cry🥺😭
But i wont give up 💪
This video is heavily edited. So all the thinking breaks are cut out and the typing is sped up. No worries, you're probably not too slow :) If you want to watch me in real time check out this playlist. Takes me 10 hours to build a freaking sidebar navigation (plus a lot of setup and stuff but still): ua-cam.com/play/PLo6qcHP9e9W5T0cwCWsQ4qcoXATqvMzcS.html
@@profydev i watch it in half speed haha, thanks!
Good communication, u are awesome
im confused about the usage of !!, i understand it does a double negation and converts to boolen, but why concert to boolean if the value is falsy anyway?
More than that, couldn't he just have used Map.has() instead of double negating Map.get()?
Can i use Map, Set, Proxy, inside a useState Hook?
You can definitely use Map and Set with useState. Proxy I'm not sure
Very helpful review. Thanks for sharing
Although I'm still not able to understand one thing
In the first line of handleOnChange method you doing this const updatedCheckedById = new Set(checkedById); (I have taken this from your blogpost , in the video it is Map)
How come this update the previous Set and if so the next if check should return true, but surprisingly it is returning false
Can you share some thought here
Apologise for my lack of knowledge in advanced
I'm not sure if I get the problem right. But let me try to explain:
1. Initially checkedById is an empty set
2. new Set(checkedById) just clones the current set. The goal is to create a new reference so we can use it to update the state (as React state has to be immutable)
3. The first if uses updatedCheckedById.has(id). Since checkedById is initially empty so is updatedCheckedById. So .has(id) returns false.
4. If an item is already selected its id is included in the set. Then .has(id) returns true and the id is removed from the set. The result is that the item is unselected.
Does that make sense?
Can you point me to resources explaining the !! on the input's disabled attribute?
The double exclamation operator is to forcibly cast a boolean value in a variable.
let something = "Test";
console.log(typeof something); // string
let otherthing = !!something;
console.log(typeof otherthing); // boolean
So basically, the !! in the disabled attribute forces the assigned variable to act as a boolean value. It's for type safety.
Also you can ignore the !!isIssueOpen at 11:24 since the variable was already a bool., he just forgot to remove it.
Can you add a live clock extension to vsCode so we can judge how long the rafactor actually took otherwise imposter syndrome starts acting up with all the time skips :D
Haha that wouldn't help either as this was for sure not the first time I refactored this code :)
I understand the convention that methods returning booleans should begin with `is`, since the assumption is that the method's object is the noun part of the clause. However, free variables are also nouns, not actions. I cannot help but feel that this convention being carried over to variables is a consequence of dogma, rather than usefulness. "IsIssueOpen" is a question, not a statement. "If is issue open" is not good English.
Does anyone have an argument to counter this?
10:07 idk much about react, but readability was easier. Now Id make a comment above about the false states.
How did he delete the whole line at once??
Probably ctrl x without selecting anything. Then the current line should be gone.
You people need to use the vim extension. If you think deleting a single line is impressive, vim will blow your fkn mind
has-method fits here better 5:45
Thanks, good catch. It's even better to replace the Map with a Set (suggestion from someone on Reddit). I adjusted the code in the blog post (see the description). But unfortunately, I can't update the video anymore
bitflag, where?
Could have used a set
Thanks for the feedback. True, somebody on Reddit pointed out the same thing. The blog post that's linked in the description now uses a Set. Unfortunately I can't update the video here.
Very cool content but a bit too fast IMHO. 👍
I feel bad I just don’t understand
Those are some long variable and function names lol
Wow Sar, how to become pero laik u
Mmmore of. This please
WTF is with the bot comments?
Love it, thanks from sharing.
Thanks for watching!