Hacker News new | ask | show | jobs
by flabbergast 2472 days ago
> I’d probably fire someone if I started to see `JSON.parse(…)` everywhere in a codebase just for “performance reasons” …

Yep, and I'd fire you for doing that! There are better ways to manage instead of showing off your authority. Oh, and by the way, would some JSON.parse statements for performance be the worst thing in your codebase(s) you guess? I mean, I cannot believe that would be the worse in your codebase. Also, if it really helps to use some JSON.parse for creating big objects for performance reasons, who cares? Instead of firing 'someone' maybe you can add some annotation to it for readability (or if that is below your imaginary level, ask the developer if he/she can add that).

Sry, but I hate people that misuse their authority by imposing their subjective opinions.

6 comments

You're extrapolating quite a bit from a simple comment, which tells me that you'd probably be a poor manager as well. Then again, I'm extrapolating quite a bit as well.

Seeing something like JSON.parse throughout the code is definitely a code smell and could decrease the maintainability of the codebase, and that's a very tangible problem. Obviously you shouldn't fire someone over something like this if it's the first offense, but it definitely raises red flags and should make you monitor things a little more closely. If they show a pattern of dogmatism and poor judgement, you're probably better off finding someone else with better judgement. You're not going to find a perfect employee, but some employees are just better at making decisions for a larger project than others.

"We apologize for the fault in the subtitles^Hjavascript. Those responsible have been sacked."

"Those responsible for sacking the people who have just been sacked have been sacked"

"The directors of the firm hired to continue the credits after the other people had been sacked, wish it to be known that they have just been sacked."

Relevant "The IT Crowd" scene: https://www.youtube.com/watch?v=pGFGD5pj03M
Interesting how typescript plays into this - I mean back in the wild old days of plain old JS I would be totally fine with putting a JSON.parse here and there, especially on the hot path.

But now with static types - this would totally wreck static type checking. And you would need to spend additional cycles to validate that the data is actually correct.

Definitely a change request in the PR.

This has to be probably a really big validate perf advantage to warrant the loss of static checks.

Typescript gives you type checking if you import from a JSON file. (Node handles JSON imports and webpack will happily build that for you into a JSON.parse in a bundle.)
To be honest I think it's a big mistake on the part of typescript to not have a JSON.parse<T>.
That would just obscure the lie. I'd rather see an explicit `as T` cast at the call site to make the "trust me, typechecker, I know what shape this is" claim be in-your-face instead of hidden behind a type parameter.

(This reply assumes you're not asking for TypeScript to make a major philosophical shift and start generating runtime code to validate types. If you are, that's a discussion worth having but goes way deeper than `JSON.parse`.)

Can JS static typing not evaluate constant expressions to infer types?
TypeScript has great type inference, but there's no way to get it to parse JSON strings at type-checking time.
> Oh, and by the way, would some JSON.parse statements for performance be the worst thing in your codebase(s) you guess?

One thing I have seen from managers who don’t work regularly in the codebase — they tend to over-focus on things like whitespace and function names more than correct abstractions, separation of concerns, etc.

Would you also fire yourself?
That’s the compiler/minifier’s role anyway, to use the best construct when appropriate.

See Java’s whole “abc”+”ced” vs StringBuilder performance issues. When programmers have to alter readability for performance, it doesn’t necessarily mean they shouldn’t do it, but it means the precompiler is not advanced enough.

I wish could upvote this more than once.

Readability is crucial in code. If you have to through and change the JSON that's being parse and it takes a nontrivial amount of time, that's a big setback. Sure, it's 1.7x faster (in v8) to parse JSON, but how long does it take to parse 10kb of an object literal in the first place? Given that these static, large objects are not common place in a codebase, is it worth the tradeoff?

The precomiler, such as Babel, could introduce a plugin for this sort of optimization. We only write ASM when it going to significantly change the performance characteristics, and typically when a particular code path is run many, many times throughout an application. If an object literal like this is getting parsed that frequently, there are better ways to optimize so that doesn't need to happen at all anyway.

I could see this being very useful in a variety of applications, such as server side rendering. However, its would be best to happen in an optimization phase as you're already bundling at that point.

That was a weird era in Java history. They changed the compiler in the subsequent major version to perform that transformation automatically, but by then people had had 2 years to stare at perf graphs looking for bottlenecks.

It never was clear to me why they didn't do both of those in the same release. Backward compatibility wasn't the problem (they were already breaking that left and right).

Even better, I think Eclipse’s Java compiler introduced the optimization in a given version, but Maven hadn’t yet. So it wasn’t optimized in production, but was optimized on the developer’s machine. What a time to be alive.
Java devs saw that "abc" + "def" involved expensive String concatenation, so as a performance improvement they pro-actively, and effectively manually, changed to use explicit StringBuffer concatenations.

When the compiler switched to generate StringBuilder (unsynchronized) concatenations for "abc" + "def" nobody benefited, because they had already changed to use StringBuffer (synchronized).

Now they had to go an undo all of their hard, manual, optimization work.

I feel like the same would/might play out here.

define: hyperbole

noun exaggerated statements or claims not meant to be taken literally.