Hacker News Reader

Comment by KMag | Hacker News Reader

A couple of related practices I've picked up over the years: (1) using long names when implementing things that are necessary but discouraged (2) putting dange_danger in names of things that should never be used in production.

The former introduces just a tiny bit of friction, but it's often enough to encourage developers to use the preferred API calls.

The latter I learned almost 20 years ago from Google's "Mustang" search back-end. If running a test instance of Mustang without bringing up a ton of dependency processes, there was a --danger_danger_must_not_fail true commandline flag (or something very similar) to turn a bunch start-up sanity check aborts into warning log messages. If any code reviewer sees "danger_danger" being added to production command line flags, they'll almost certainly ask the right questions.

View on HN

The Haskell ecosystem uses a very similar convention: unsafe functions have names starting with ‘unsafe’ (or, in one particularly horrible case, ‘accursedUnutterable’ [0]). These are generally functions which subvert the type system, so the name acts as an alternate warning.

[0] Documented at https://hackage.haskell.org/package/bytestring-0.11.4.0/docs... — incidentally one of the funniest pieces of documentation I’ve ever seen!

That doc link is funny, but it's also very frustrating -- you would think that after all those warnings, there would also be some glimmer of information about what sets the accursedUnutterablePerformIO function apart from unsafePerformIO, or at least why it exists, but apparently not. It's clearly important because most of the core functions in Data.ByteString call it.

It seems like the authors have decided that the trickiest, most magical parts of their codebase are exactly where they don't care about useful documentation. That seems like a bizarre strategy unless they plan to never let anyone else help with maintenance.

“If you aren’t willing to read the source of the function to learn what it does, we don’t want you anywhere near it. No summary short of the function itself can accurately communicate how much of a fragile hairball of edge-cases this function is, and how many things you have to know about to correctly call it.”

This is pretty much correct. For reference, here’s the original documentation (changed in commit 80ff4a3018cd8909abb1d4e0c32f012a523883ec):

    -- | Just like unsafePerformIO, but we inline it. Big performance gains as
    -- it exposes lots of things to further inlining. /Very unsafe/. In
    -- particular, you should do no memory allocation inside an
    -- 'inlinePerformIO' block. On Hugs this is just @unsafePerformIO@.

The commit message notes that ‘We've had a few instances of people being tempted to use it without really understanding the consequences’.

I wonder why inlining the function makes it that much unsafe.

This is the worst kind of arrogance. All you're doing is reducing the number of people who'll understand what's actually going on, which not only makes it less likely to be used properly; it also makes debugging code that uses it more difficult.

You're deciding for other people what they should and should not know. Another word for this is gatekeeping.

Don't do that. You're not the other person. You can't read their minds, nor can you fathom their reasons. Deliberately hamstringing folks is bad policy, and supremely arrogant.

Just document it properly, lay out the risks, and leave it to the reader to decide on policy.

Oh wow your comment really rubs me the wrong way...

In my opinion, it is properly documented, almost perfectly so, by the authors giving you the full source code.

If you are not willing to read the source code, but instead always blindly rely on documentation to be correct, then in my book that's just lazyness.

> If you are not willing to read the source code, but instead always blindly rely on documentation to be correct, then in my book that's just lazyness.

Hmm, I agree somewhat with both your comment and the one you're responding to. However, the claim that you should read the source of the libraries/frameworks you're using to me is interesting - because to me it seems sane, however not always entirely viable.

For example, who would be more successful:

A) A Java developer who reads the Spring source code and dives into that verbose Eldritch codebase with years upon years of fixes, design patterns and abstractions, all to learn how the dependency injection works better, probably spending days to weeks in the process?

B) Or perhaps someone who looks up the annotations or configuration they need, or maybe other code snippets on StackOverflow or the docs, or a similar site and solves their problem so they can move on to the next tasks, while being none the wiser otherwise?

Sometimes the answer will be A (deep knowledge can be useful), sometimes it will be B (we all have limited time and energy), however it feels to me that with sufficiently complex codebases one might just get more and more confused as well, as opposed to gaining any sort of a clarity or understanding, when the framework/library actually abstracts away really complex things.

Just using Java as the example here, as it's one of the more enterprisey languages and Spring as an arguably brownfield maintenance mode framework that's huge, given its long history.

I appreciate that you broke apart the users into two groups. I imagine Group B is much larger than Group A, and that the Haskell maintainers want to emphasize "if you use accursedUnutterablePerformIO, make sure you are in Group A!".

Most Group A people didn't get there fixing surface level stuff. They got there because there was an insidious defect buried deep that required that deep knowledge to understand or fix.

Documentation is not a substitute for reading the code, but rather a complement to it.

Code by itself is a limited medium that can only do so much. Asking someone to rely solely on code to explain a complicated subject is counterproductive (the more complicated the code is, the harder it is to guess whether something in the code is a mistake, or working as intended with some unknown reasoning behind it).

Documentation points out the non-obvious parts, the reasoning behind it, the subtleties, the gotchas (things to watch out for), the best practices, etc.

Documentation is a map to guide the user, not an end-all-be-all about what the code does.

When the code does non-obvious things or has non-obvious reasons, you need both code and documentation.

I would agree in general, but the comment I was replying to is criticizing the following documentation as arrogant:

"If you aren’t willing to read the source of the function to learn what it does, we don’t want you anywhere near it."

And I believe that might be reasonable for a complex function that cannot be exhaustively documented in any form shorter than the source code.

I disagree, but I think this comes down to the wording and tone.

The following would be perfectly justifiable and I'd agree with it wholeheartedly:

"This code is incredibly complicated and we don't have the resources to document it properly. So please DO NOT GO ANYWHERE NEAR THIS CODE unless you've spent weeks/months studying it in depth. And even then, think twice!"

One is a friendly "here there be dragons" style warning that explains some things (such as why it's not documented). The other is a passive-aggressive accusation followed by an "I know better than you, kiddo" style enforcement, which exudes arrogant presumption.

But that phrasing makes an entirely different point. It's not "we don't have the resources to document it properly" — it's more like...

"There is no possible documentation that could help you to not screw up when using this function. We've tried. There used to be docs here — first as doc comments, then hidden within the function (to force you to read the function to find them), then as a special pre-commit lint rule that triggers when people add a new use of the function to the codebase without adding a special annotation to acknowledge it. None of it helped; people still screwed up the usage. Even the people who wrote the function screwed up in new usages of it — every time they tried! — because the number of things you need to remember to get right to use it, exceeds the capacity of any single human being's short-term working memory. The very existence of this function is Ozymandian arrogance. It should be banished from the Earth. The only problem is that people keep reinventing it. We're waiting for a sufficiently-smart linter, so we can teach it to not let people reintroduce any function isomorphic to this to the codebase; then we can truly excise it for good. In the meantime, we leave it undocumented, for a bit of security through obscurity: a thing that you can't find a good way of coming to a seeming understanding of, you're less tempted to use."

I'd argue that both of you are somehow correct. Give me the code, so I can see what it does. And give me documentation (be it correct comments, a dedicated documentation or whatever) to know why it does something.

Code states the obvious, but you cannot read between the lines.

It reminds me of how narrow roads are sometimes used to slow traffic, though I dont know if its effective here. Im hesitant to discourage it when the language is so explicit about it being bad, it might be quite effective afaict

Your fallacy is:

https://www.logical-fallacy.com/articles/false-dilemma/

It's documented that you need to read the source.

Good documentation tells you a lot more than the source could. Especially when you're talking about horrific side-effects.

Another correct way to use Hungarian Notation, according to (iirc) Joel Spolsky

To me unsafe generally means “memory safety or type safety is disabled”. It doesn’t mean the code can’t run in production. Maybe some just implemented fast inverse square root and tested it manually.

danger_force_electrify_fence() is a whole different story.

> To me unsafe generally means “memory safety or type safety is disabled”.

And that is exactly what it means. Unsafe functions in Haskell do run in production. They just subvert type safety (and I believe memory safety too, in the case of accursedUnutterablePerformIO).

Even plain unsafePerformIO is memory-unsafe, as it will let you construct a polymorphic IORef, through which you can implement unsafeCoerce.

Hmm… is that memory-unsafe? I thought that came under the heading of ‘subverting the type system’. Then again, I suppose they’re more or less the same thing in a strongly-typed and pure language like Haskell.

Yes, it is very memory unsafe. Subverting the type system often the main source of memory unsafety in most languages. It typically means you can treat an arbitrary integer as a pointer to any other object.

Rust and C# both use 'unsafe { }' blocks, which can only be enabled by a module-level compiler flag.

There is no such "compiler flag" in Rust, that's only a C# thing.

But also more relevantly here, stuff you'd be calling in those Rust blocks tends to have longer names and often calls out the fact it's specifically not checked, e.g.

  a = b.unchecked_add(c); // Like a C or C++ arithmetic operation if you overflow it's UB. This might be faster. It might not. But if you must have this anyway, that's how

  n = NonZeroU32::new_unchecked(0); // Unlike new() which returns None because duh, zero isn't non-zero, this results in UB.

Not all of them though, for example:

  v = Vec::from_raw_parts(ptr, len, cap); // Make a Vec, the pointer ptr had better actually be pointing at contiguous memory for exactly cap item-sized slots, the first len of which are in fact legitimately values of the appropriate type, if any of this is wrong that's immediately UB.

While sadly it does not require a flag to enable, at least you can turn it off with: `#![forbid(unsafe_code)]`.

For languages that support Unicode identifiers, you could zalgo-ify the name to increase the friction:

d̜̣͎̱̟̥͓̥͍̙͖̫̓̀͗̌̚a͍̩̫̳͖̒̒́́͐̍́̍̏͋̿̈n̳̯̫̥̞͗̒̆͗̀͒͒͌̒̎g͉̝̠̝̠͉̰̘͉̩͖͙̾͐̽̂̈́͗ë̗̙̳̮̘̟͙͇̣͇́̐͑̈́͂̾́̇̇̀͒ŕ̗̱̜͚̌̔̓̄̄͒́̄͌̚_̱̭͍̣̳̝͓͔͖͓̀̃͒̑̊͛͛̽̑͋̿͒ͅd͉̰̖͎̰̙̘̰͈̫̃̍́̋̆̀̓͆̈́̓̇̾a̭̦̲̫͕̩͔͙͔͖̱̿̑͌̊͌̾̊͂́͂̉ň͖͔̩͓̮̦̭̜̩͉́̀̍́̑g̖̦͙̝̈͆͗̈̄́̉̽͗̄͆́ȅ̠͔͕̜͔̇͗̊̈́r͎̬̦͈͈͐͑̃̄̀͒̉̓̋_̪̝͔͖̠̘̰͛̂́̔̆́̓̅͒t̖̘̟̝͚̭̦̪͕̘͐̎̋̈̍̋̾̈́̇̈́̉̚ḣ̙͇̝̳͓͇̮̫͙͈͍͊̇͌e̳̖̯̥̭̜͓̜̒̉̎̒_̥̱̲̘̞͓̖̣̝̮̮̌͌̅̀͊̅̍̑̿̂̿ͅp̗͖̘̝͈͙̫͖͍̠͑̎̃̍͌̇͋̍͋͗͑͛o͉̬̗̠̝̗͈̟̘̱̬̫͗̽͆̊́̂̚n͚̪̰͉̪̟̈́͛̓͒͌̎̑́̒̃͌͆ͅy̞̗͖̗̾̂̿̿̀̊͋͐̓̓̚_̜̲͖̣͈̞͖͎͆͑͌̾̃͆ͅh̦̘͇͍͓͕͒̎̒̽͂̊̐̍e͎̰̲̥̱͍̗͉̞͔̦̬̊̌͆́_̲͚̮̮̙͇̠̮͖͐́̎͆̿̄̋c̙̩͈͇̩̯̠͇͙̄̄̒̄̓͑͂͗̈͂̊̚o̪̥̲̗͓̭͍̣̮͙͉͇̽͊̿̔m̫̖̘̰̆̊̒̃̔͛͛̚ẻ͓̭͓̗̙̳͖̭͚̐̀̃́̃s͖͈̬͚̜͊̐͗̓̂͑̀͑̔̿͌͒

(Yes this is actually a valid Unicode identifier.)

https://en.wikipedia.org/wiki/Zalgo_text

Now that I think of it, a more toned-down version could be:

    d͓ͨa͓ͣn͓ͮg͓ͤe͓ͣr͓ͭ_d͓ͨa͓ͣn͓ͮg͓ͤe͓ͣr͓ͭ_f͓ͨo͓ͣo͓ͮb͓ͤa͓ͣr͓ͭ_o͓ͨp͓ͣt͓ͮi͓ͤo͓ͣn͓ͭ

And it's a perfectly valid function name in Ruby: https://replit.com/@brandt1/WatchfulTemptingRecursion#main.r...

Also works in .NET, although Visual Studio really doesn't like the identifiers.

My own example from a decade ago: https://sourceforge.net/p/portableapps/launcher/ci/default/t...: you can’t run PortableApps.com apps from the Program Files directory for a few reasons, but for some reason we decided to allow overriding the guard anyway, but in a clear and verbose way: you have to set an environment variable named IPromiseNotToComplainWhenPortableAppsDontWorkRightInProgramFiles to “I understand that this may not work and that I can not ask for help with any of my apps when operating in this fashion.” (case-sensitive).

At my old job we had some unsafe-ish operations, mostly you needed to do --force or --really. Typically you would invoke without it, then it would say "heres what I am gonna do, do this again with --really and I will do it for sure"

There was one tool that had '--yolo', because if you're at that point of an operational crises - you might as well give it a shot.

I too have written force features. You've likely never seen this, but in tool design, prefer git push's --force-with-lease design.

Force With Lease says "I believe X is true, and because of that, force". The tool can check that X is indeed true, and if so force, but if it's not true the human was wrong and they ought to re-consider and obtain a new "lease" before we make the change.

In Git this "lease" is the current ref of a remote, if we specify this, but actually it's wrong, that means the state of the remote system has changed, and we need to re-consider whether our forced change is still appropriate. e.g. While you and Bill were quickly changing colors.js to hackily disable dark mode, turns out Sarah guessed the actual bug, and replaced main.css with a patched version that works fine even in dark mode, if you force-push your change, instead of zapping the broken change it zaps Sarah's fix!

This approach works best where you can actually take some sort of "lock" and avoid clashes at the tool level, but there's some benefit even without that at the human level.

--force-with-lease is great, but I really wish it was --force and the current force was --force-without-checking-if-its-safe or something. The more dangerous one being the shorter one is unfortunate, and I think it's worth the breaking change to swap them (with some sort of config option to gradually do the switch).

An even better design is to have the command write out essentially a script file for what it is about to do, then the —-force flag would execute that script rather than start from scratch. That way you avoid having things come up between when you do a dry run and the real deal. This isn’t always applicable but sometimes can be a really nice pattern.

this works well for things that are shell scripts, and we certainly had at least one " check the output and then pipe it into sh". But for the most part that falls away when you have a lot of objects and abstractions and stuff.

I am going to have to steal --yolo. Love it. At a past job it was basically our team slogan (extremely tongue in cheek... has anyone ever said YOLO seriously?) any time we were deploying a change we were at all nervous about. I can't believe we never codified it into our tooling.

It's originally called "Carpe Diem", and it's been a common phrase used and misused for millennia

Of course, no one thinks that when doing something truly dangerous.

Many people have definitely said "you only live once" and meant it seriously.

Yolo became a joke came AFTER people did a bunch of stupid shit.

Kind of like "hold my beer".

YOLO was the motto in the popular Drake song “The Motto”…

Was this old job Canva by any chance?

For 1, I have seen somewhere that it a reason why C++ casts are so verbose compared to C casts: like "reinterpret_cast<A>(b)" vs "(A)b". Type casting in C++ may be the result of bad design. And while casts definitely have a place, they are ugly, and therefore, they should look ugly.

But C++ still lets you use C-style casts, which kind of defeats the point.

Not so much, you could phase out C-style casts over time. C++ compilers could grow options to warn about C-style casts (if they haven't already), shops could outlaw them in coding style guides, and slowly it would go away.

What does kinda defeat the point is that C++ adds a new form of cast that is even less conspicuous than a C-style cast:

  typedef int* int_p;
  int f(char* p)
  {
      int* ip = int_p(p); // basically a reinterpret_cast
      return *ip;
  }
  int main(int argc, char** argv)
  {
      return f(argv[0]);
  }

gcc -Wall compiles this with no warnings.

Ugh, I really need to clean up and submit my patch that adds those warnings. Hopefully I'll have some time this year.

You can blanket ban C-style casts with appropriate warnings and static analysis.

This is the flag required by our script to create and merge pull requests from one environment to another:

MERGE_APP_REPOS_YES_I_ALREADY_MERGED_LIB_REPOS_AND_WAITED_FOR_THEM_TO_FINISH_BUILDING

This seems like a broken process to me.

It's a script run by a human when they want to push the changes in one environment to another. Our various apps consist of 3-10 repos. All the apps are backed by two shared lib repos (one for node lambdas, and one for C# lambdas). Merging each repo manually to do an environment push is cumbersome.

The script creates and merges pull requests on github for the lib repos and app repos - from one environment (branch) to another. First you have to do the lib repos, and if there are any changes wait for the CICD jobs to finish. If there are no changes to the libs, the script indicates that no pull requests were needed.

I have no idea how to get a local bash script to poll for when AWS CICD jobs are done, nor do I really want to spend days on that. So I just added that parameter to make sure I (and anyone following me) runs the lib merges first and waits for them to finish building before merging the app repos. The job without any flag just merges the lib repos.

Next time there is a HN thread about monorepos I think I'll just posts link to this comment.

On small fixes, it comes in handy to be able to separately deploy front end code, API Gateway code, or different back end lambda groups. But other times, like big feature additions, most of the repos have new changes, and I need to push an entire environment.

Part of this is an artifact of using lambdas and serverless.com. Serverless deploys all the lambda code in a repo to each lambda. It doesn't try to parse which code is shared. If I add too many lambdas to a repo, deploys take longer and I run into the AWS lambda code size limit.

But I also don't want a repo for each lambda because then I'd have to automate a ton of stuff that's currently manual, and for our tiny shop I think that would be overkill. We'd almost need a dedicated automation person.

We also have repos for several react front-end web apps, and the AWS API Gateway code that connects the front end to the back end. It's very common that I only deploy the react web-app. I have no idea how I would do that in a mono-repo that had all the front end, API Gateway and serverless.com/lambda code. I guess git tags? But I'd always be worried about screwing those up.

Why not create a draft PR on the app repos and tell user to merge it when lib builds are done?

It would be the same result. A human still has to wait for the lib builds to finish and then take action. This way there are two fully automated steps, with one human wait step in between them.

Yeah, but it will be a simpler script though.

Seems like that is intended to never be used by humans and is used by a process that has always done the other thing first (unless you're manually debugging the process itself in which case you'll copypasta the silly long thing).

if_you_use_this_you_will_be_fired is my favourite suffix seen in production.

in react, I assume

https://github.com/facebook/react/blob/80bff5397bf854750dbe7...

It's used in many of Facebook/Meta's internal code bases, in fact.

And it has been for a long, long time

https://twitter.com/thefotios/status/689130197891829760

This comes with a huge caveat: You must explain it in enough detail for anyone encountering it to understand why it's dangerous and what they should be doing instead, if at all possible. The documentation must at the very least be where the name is defined. "But why?!" is exactly the wrong thing for a developer in a hurry to be thinking.

my contribution: public_method_that_only_deep_merge_should_use

https://github.com/chef/chef/blob/68dd5f42273f19bc5975c0dc8e...

that was 9 years ago and it was code smell that things were broken apart incorrectly and at some point i rewrote it so that wasn't necessary -- but sometimes you just gotta move the ball down the field, even if you don't get a first down.

I have a similar insight regarding long names.

If you strive to make your function names as self-documenting as possible, you then end up with the following situation: either

1) Long-named functions become a signal that code structure is suboptimal and can be refactored to split functionality / responsibilities appropriately, until the function has a reasonable name, or

2) You are working with code that cannot be refactored, in which case the long, cumbersome, but otherwise highly informative name is doing you a favour, as well as possibly forcing you to wrap meaningful interfaces around it to make you less reliant on calling that monstrosity directly as much as possible.

So yes, I always try to make my function names contain as much relevant information as possible.

Yep - we do the same thing: functions which don't (for example) check if a given user should be able to do the given thing use then naming pattern dangerously_verb()

i wrote "this is dangerous" in a comment the other day and then it failed on me the next day. whoops. i guess i was right

Reply on news.ycombinator.com