Refactoring A Junior’s React Code - 43% Less Code With A Better Data Structure

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

[@MM-pw4ft]

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

[@diogoribeiro477]

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.

[@profydev]

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 🙂

[@ogmandarkian]

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..

[@ploonkey]

This is great, would love to see more of these.

[@Ss-io9qk]

Great stuff! I've definitely been sleeping on the usefulness of maps! Subscribed :)

[@piyushaggarwal5207]

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. 😆

[@skapha]

Very good! Yes, I want to see more of this stuff.

[@gilbertmutai3342]

The refactoring is just on point. Good work.

[@danieledwards6133]

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.

[@tayshin.gg.]

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

[@kopfmann45]

This is excellent! getting back into React after a long break, and your videos are a godsent. Keep it up!

[@profydev]

Thanks :)

[@amantulsyan1082]

This is was amazing, would love to see more content like this

[@PraveenKumar-xd7ix]

Great work brother . Expecting more like this 👍👍

[@mohammedsamir5142]

Keep doing these awesome videos. I find them so helpful!

[@Den368]

Nicely done! As a senior developer I can assure that your work here is a gem.

[@profydev]

Thanks, that means a lot

[@franciscoizaguirre9069]

I wish I saw this YEARS ago, that was awesome and insightful

[@irondsd]

Awesome. Would like to see more videos like this

[@miceyjuice9175]

I love these kinds of videos! Great stuff!

[@JulietteReacts]

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

[@profydev]

Thanks so much for the feedback!

[@Gelu84]

Great video! I would love to see more of these

Great video, really helpful for someone getting started!

[@kewtomrao]

This Is really helpful as a junior dev myself!

[@rajaj6043]

Awaiting more video like this, Thanks for posting!!

[@NoopDawg]

This is wonderful, this kind of content is very helpful

[@barhoumiahmed179]

WE NEED MORE OF THIS CONTENT

[@tellyisko6908]

Subscribed! I want more of this. 🤩

[@okage_]

thanks for this, helps a lot as a junior

[@BCRooke1]

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.

[@BCRooke1]

Never mind I see you addressed this in another comment. I didn’t see that until now! Thanks

[@TommasoManca]

Awesome stuff, would definitely love to see more of this. Liked 'n subscribed!

[@samareshdas767]

Thanks. As a just learned react developer, this will help me

[@DomskiPlays]

Wow this video was actually amazing, that's wild!

[@dyunior]

Hi, hope you will have more of this video series, or refactoring playlist. Thanks for the content! thumbs up!!

[@profydev]

Thanks for the feedback. Check my React Architecture playlist. That's basically a larger refactoring as well only with a focus on architecture

[@Stragunafen]

Loved this kind of video!

[@developer_george]

Thank you for this video! Helpful 💓💓

[@psatler]

Really cool video! Thanks!

[@ssumit196]

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.

[@yt-sh]

I subbed, hoping to see more refactoring videos!!

[@tonienguix4834]

aah I could watch these all day. Loving it! Very tempted to send my code... hoy could I do that? Cheers!

[@ToadyEN]

Juniors code is better than some of the seniors where I work.

[@ra-dro]

Thanks for sharing. Cool work. One thing I could suggest is to split component into several smaller once.

[@profydev]

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

[@idoschacham6276]

Excellent refactor, well done

[@bao3576]

Very informative!

[@blaycoder]

I would love to see more

[@stephaniemay9090]

more videos like this please!

[@fesci009]

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 😅

[@nelsieb.1366]

Nice! Would be interesting to see how the solution can be extended for a filterable list

[@Famouzi]

Great video :)

[@mrlectus]

Last issue is not using typescript

[@a7mdbest15]

thank you very much for the video

[@fabrzy3784]

Amazing video ! May I ask when is it appropriate to use a map vs an array?

[@bertrodgers2420]

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

[@MichaelKire]

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.

[@anonymousanon4822]

​@@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.

[@anonymousanon4822]

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)

[@itsMapleLeaf]

good video! I would make a habit of saying "simply" less often. what's simple and obvious to you might not be to others!

[@kimballle7714]

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

[@sarcasticdna]

Do it just do it this is awesome

[@BlackxesWasTaken]

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

[@Tom_Rose]

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! :)

[@profydev]

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 [e-mail]

[@Tom_Rose]

@@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?

[@profydev]

@@Tom_Rose Sure, feel free to send me an email to [e-mail]. There's no need to share the whole code btw. A reduced version on CodeSandbox would even be better

[@d_3677]

Boss level♥

[@piyushaggarwal5207]

Unnecessary state variables are the worst.

[@moveonvillain1080]

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.

[@SpikeYOLO994]

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

[@SpikeYOLO994]

Nvm , I didn't get to the part where you mentioned about memory :D , great job XD

[@profydev]

Haha thanks

[@luiza4142]

Can i use Map, Set, Proxy, inside a useState Hook?

[@profydev]

You can definitely use Map and Set with useState. Proxy I'm not sure

[@flalspspsl6858]

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.

[@profydev]

Thanks for the hint

[@tyu3456]

Doesn't Node.js also use V8?

[@grenadier4702]

has-method fits here better 5:45

[@profydev]

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

[@Seth_H_Builds]

Can you point me to resources explaining the !! on the input's disabled attribute?

[@sjn_]

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.

[@ploonkey]

Also you can ignore the !!isIssueOpen at 11:24 since the variable was already a bool., he just forgot to remove it.

[@LaFragas]

Omg u do it all so fast compared to me, it makes me cry🥺😭 But i wont give up 💪

[@profydev]

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): kzbin.info/aero/PLo6qcHP9e9W5T0cwCWsQ4qcoXATqvMzcS

[@LaFragas]

@@profydev i watch it in half speed haha, thanks! Good communication, u are awesome

[@PetersExcapades]

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?

[@Igstefano]

More than that, couldn't he just have used Map.has() instead of double negating Map.get()?

[@shovandhara5976]

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

[@profydev]

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?

[@reikromulis7747]

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

[@profydev]

Haha that wouldn't help either as this was for sure not the first time I refactored this code :)

[@samarrowsmith2723]

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?

[@Deutschehordenelite]

10:07 idk much about react, but readability was easier. Now Id make a comment above about the false states.

[@jayeshcholker]

How did he delete the whole line at once??

[@profydev]

Probably ctrl x without selecting anything. Then the current line should be gone.

[@SogMosee]

You people need to use the vim extension. If you think deleting a single line is impressive, vim will blow your fkn mind

[@xav_624]

Very cool content but a bit too fast IMHO. 👍

[@user-uo3ox5hf6u]

Could have used a set

[@profydev]

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.

[@Hacking-Kitten]

bitflag, where?

[@mateito191]

I feel bad I just don’t understand

[@cyrusren7260]

Wow Sar, how to become pero laik u

[@sjn_]

Those are some long variable and function names lol

[@nested9301]

Mmmore of. This please

[@jeremykothe2847]

WTF is with the bot comments?

[@meaningmean]

Love it, thanks from sharing.

[@profydev]

Thanks for watching!