Code Review with a Jr JavaScript Developer - Here's What I Found!

I made these 5 suggestions throughout a 2 hour code review with a junior JavaScript developer.
*Newsletter*
Newsletter 🗞 - www.jamesqquick.com/newsletter
*DISCORD*
Join the Learn Build Teach Discord Server 💬 - / discord
Follow me on Twitter 🐦 - / jamesqquick
Check out the Podcast - compressed.fm/
Courses - jamesqquick.com/courses
*QUESTIONS ABOUT MY SETUP*
Check out my Uses page for my VS Code setup, what recording equipment I use, etc. www.jamesqquick.com/uses

[@jesper.ordrup]

I like to decontstruct variables but you could argue against. If you dont deconstruct you automatically get a good descriptive name: subscriber.email says a lot about which email you refer to.

[@keithjohnson6510]

Spot on, I would say avoid using them just because there cool.. :) But what worries me more for James is the advice on error handling.. :(

[@jesper.ordrup]

@@keithjohnson6510 Thanks. I also encourage this approach when importing modules

[@SonAyoD]

@@keithjohnson6510why?

[@gosnooky]

The joke is the two hardest things in programming is cache and naming things.

[@deadlyecho]

Well, I am pretty bad at naming things 😂😂

[@jlmx]

And off by one errors

[@jlmx]

The joke being that you list three things

[@sujoykrhaldar]

Learned something new regarding err handling,
throwing err as obj key and use it for checking a condition.
Thank you.

[@benpluim7685]

Loved this! Especially the view of typescript at the end. Im just starting a new role in a few weeks and I'm always looking to improve my code so that reviews are seamless!

[@gregmunro90]

Thanks for the video, very useful. What vs code theme is this?

[@SeRoShadow]

1. Object Properties 0:27
Never check if an object has a property by accessing its value:
If it doesnt have it, the code wont run ; GOOD
if it has it and is nullable (undefined / null) , the code wont run ; BAD ; sometimes nullable values are expected and this is true especially when fetching data
use hasOwnProperty method on the object instance instead or iterate over the project keys to find the match, then code accordingly.
personally, I prefer a custom function, hasOwnProperties() , that takes in one value, primitive or array, then checks for matches in object props for every value.
2. Using shorthand notation 1:57
I disagree with that since if the code will ever require refactoring, the changes will have to cascade all they way, otherwise the code will not work properly.
I personally write the longhand notation so that code style is consistent and the small changes wont require refactoring the entire codebase

[@keithjohnson6510]

1. I tend to use `in`, mainly because it helps with Typescript , type narrowing.
2. I wouldn't say I disagree with shorthand notion, but would agree that use it with caution. In other words, don't use it as cool factor, if using shorthand notification loses intent, then you have made harder to debug code.

[@RobertMcGovernTarasis]

Curious, with autocomplete being a thing, isn’t it better to be explicit with subscriber.email vs simply email. Keeps it clear where that variable comes from?

[@hunterbertoson156]

There are 2 hard problems in programming: cache invalidation, naming things, and off-by-1 errors.

[@jlmx]

Return types FTW 💪

[@nidinpereira5898]

Destructuring has its downsides, if email is undefined, everything is going to break down.
Doing a safety check first is always better, even if it requires writing a few extra words

[@keithjohnson6510]

That is true, but if say using Typescript like James is doing and `email` has come from a type that's not optional, then that can't happen, and if it was optional the nice thing with Typescript it would warn or error for you too. I would say that is one of the nice things with Typescript, sanitising data only needs to be done once, passing data down the call stack will have implicit type checking, saving on extra checks.

[@WebDevCody]

Soon you’ll be writing go