| ▲ | q3k 10 hours ago |
| 1. Randomly peeking at process.argv and process.env all around. Other weird layering violations, too.
2. Tons of repeat code, eg. multiple ad-hoc implementations of hash functions / PRNGs.
3. Almost no high-level comments about structure - I assume all that lives in some CLAUDE.md instead.
|
|
| ▲ | delamon 10 hours ago | parent | next [-] |
| What is wrong with peeking at process.env? It is a global map, after all. I assume, of course, that they don't mutate it. |
| |
| ▲ | lioeters 8 hours ago | parent | next [-] | | > process.env? It is a global map That's exactly why, access to global mutable state should be limited to as small a surface area as possible, so 99% of code can be locally deterministic and side-effect free, only using values that are passed into it. That makes testing easier too. | |
| ▲ | hu3 9 hours ago | parent | prev | next [-] | | For one it's harder to unit test. | |
| ▲ | withinboredom 8 hours ago | parent | prev | next [-] | | environment variables can change while the process is running and are not memory safe (though I suspect node tries to wrap it with a lock). Meaning if you check a variable at point A, enter a branch and check it again at point B ... it's not guaranteed that they will be the same value. This can cause you to enter "impossible conditions". | |
| ▲ | q3k 9 hours ago | parent | prev [-] | | It's implicit state that's also untyped - it's just a String -> String map without any canonical single source of truth about what environment variables are consulted, when, why and in what form. Such state should be strongly typed, have a canonical source of truth (which can then be also reused to document environment variables that the code supports, and eg. allow reading the same options from configs, flags, etc) and then explicitly passed to the functions that need it, eg. as function arguments or members of an associated instance. This makes it easier to reason about the code (the caller will know that some module changes its functionality based on some state variable). It also makes it easier to test (both from the mechanical point of view of having to set environment variables which is gnarly, and from the point of view of once again knowing that the code changes its behaviour based on some state/option and both cases should probably be tested). |
|
|
| ▲ | loevborg 10 hours ago | parent | prev | next [-] |
| You're right about process.argv - wow, that looks like a maintenance and testability nightmare. |
| |
|
| ▲ | s3p 10 hours ago | parent | prev [-] |
| It probably exists only in CLAUDE or AGENTS.md since no humans are working on the code! |
