TypeScript review
I have the following TS code:
I'm trying to take an array of strings and numbers as an input and convert it to a map. I got this code working, but it took me almost an hour and a half and seems incredibly convoluted for the task. Surely there's a better way?
22 Replies
Also feel free to roast this. Honestly embarassing how convoluted I made this so I would love some feedback. I converted array to map to object lol... feels really dumb
I know you can do this in like 1 or 2 lines as well I just don't know how without looking it up lol
Oh you can just use 2 for loops 😅
You can remove the type on the
input
constant, it will be inferred automatically from the value.
Same for the const convertedArray
. And I'm pretty sure you don't need the generic parameters on the map you are returning either.
As for the code itself, while map would have better performance on a huge array (multiple thousands of items) it just makes the code messier here.
Like map.set(item, (map.get(item) as number) + 1)
could be replaced by obj[item] += 1
Yes! Thank you so much. I knew the code was definitely quite shite but I wanted to try it without looking up a solution 😂
For the maxValue you can just do
Math.max(...Object.values(obj))
That's so sick you can do that
When I get time tomorrow I'll rewrite it and send it here
I'm sure there's other stuff that can be simplified, but on a phone and without syntax highlighting I don't see them
I'm not sure what the first function is supposed to do but one little trick to simplify the TS code is, as mentioned by Ragnar above, to let it infer as much as possible.
This code doesn't actually work but I suspect it's because the conversion that you are doing is somewhat off... However, from a typing perspective this simplifies things a lot.
What it says is that convertArrayToMap accepts one parameter, an array, and it's going to infer it's type without you specifying it. The
T extends whatever
is a restriction to limit T
to whatever type you want, strings or numbers in this case. It's also reusing this type for the key of the Map.
Oh, I see. Didn't know you could construct a Map using an iterable. I personally would just use a for loop of some kind, to make it more readable.TS Playground - An online editor for exploring TypeScript and JavaS...
The Playground lets you write TypeScript or JavaScript online in a safe and sharable way.
Good exercise!
As you can see most of the effort goes into removing TypeScript annotations 😅 Let it earn it's salary and infer all that it can, you only really want to give you hints when you're doing something wrong or explicitly checking for it i.e., hovering over some variable. Otherwise it gets really annoying, really fast.
Also, I'm not using the convert array to map in this particular example as otherwise you'd be looping multiple times over the same array but this is not really needed.
You should also consider what happens when an empty array is provided. If you absolutely expect something to be returned, better to throw an error early on.
I did it another way
TS Playground - An online editor for exploring TypeScript and JavaS...
The Playground lets you write TypeScript or JavaScript online in a safe and sharable way.
it could be simpler but I wanted to try to skip ahead when possible by checking if the next set of items was longer than the current one in a sorted view of the input array
and there's only 56 caracters total that are typescript annotations (excluding spaces)
Oh shoot I gotta look at all this stuff. Thanks guys! I'll get back to you sometime soon. Kind of got swamped all of a sudden with this contract work and some other work 😂
(please note that my implementation is uselessly complexe just for the sake of it)
😂 I had to paste it into vscode and use the debugger to understand what's going on. I think I understand most of it, I'll still need to review it a bit more. I need to look at Joao's too!
I wouldn't have come up with that on my own but I like how efficient it is
It should be O(n) right?
well... it might not be actually be efficient because of the sort function...
Yea that's true, I was thinking that as well. The for loop is efficient though
yeah
What's the runtime for .sort()? Is it in place? Idk how they implement it
it's at the discretion of the implementation IIRC
as long as it sorts the array it's ok
tho it must be something like O(n log n)
so... depending on the entropy of the input, the number of items and the number of different elements my emplementation will be almost always slower
because it has to sort first
Got it!
I really like this. Super simple! Thank you 🙂 Sorry it took a while for me to get around to it
No worries!