Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Self-Documenting Code

lackofimagination.org

Post reply on HN
Print view View original

101–110 of 131 posts

Re: Self-Documenting Code

#101

Post by aleksiy123 »

OptionOfT wrote: post #46
johnfn wrote: post #34

My cut: const passwordRules = [/[a-z]{1,}/, /[A-Z]{1,}/, /[0-9]{1,}/, /\W{1,}/]; async function createUser(user) { const isUserValid = validateUserInput(user); const isPasswordValid = user.password.length >= 8 && passwordRules.every((rule) => rule.test(user.password)); if (!isUserValid) { throw new Error(ErrorCodes.USER_VALIDATION_FAILED); } if (!isPasswordValid) { throw new Error(ErrorCodes.INVALID_PASSWORD); } cons…

My issue with this is that you're using exceptions for control flow. A user not being valid is expected (duplicate username). A password not matching a regex is also expected. Then, in general (not seen here as there are no types), I like to use a lot of types in my code. The incoming user would be of type UnvalidatedUser, whereas the return type of this function would be StoredUser or something like that to distingu…

I would add one suggestion/comment. Use a known set of standard error codes and not a unique error code/error type for each new situation.

Error codes are there to hint to clients/callers what action they should potentially take (retry?, backoff)

Don't make callers handle 100s of different potential error codes/types.

If the whole internet can work with 70 your app can work with less.

All of google uses less than 20

https://github.com/googleapis/googleapis/blob/master/google/...

Put more specific information in the error message or a secondary status code.

Re: Self-Documenting Code

#102

Post by jcparkyn »

Chris_Newton wrote: post #86

If I were reviewing the original code, the first thing I’d question is the line user.password = await hashPassword(user.password); 1. As a rule, mutations are harder to understand than giving new names to newly defined values. 2. The mutation here apparently modifies an object passed into the function, which is a side effect that callers might not expect after the function returns. 3. The mutation here apparently cha…

Agreed, and then there's the time of check/time of use issue with creating a user. Probably not a vulnerability if userService is designed well, but still a bit dubious.

Re: Self-Documenting Code

#103

Post by 110jawefopiwa »

Izkata wrote: post #99

I'd like to propose weird alternative to this: function throwError(error) { throw new Error(error); } async function createUser(user) { validateUserInput(user) || throwError(err.userValidationFailed); isPasswordValid(user.password) || throwError(err.invalidPassword); !(await userService.getUserByEmail(user.email)) || throwError(err.userExists); What if... [ [() => validateUserInput(user), err.userValidationFailed], […

I assume this is satire, but for those who might take this seriously, please avoid doing tricks like this.

You're doing so much extra work here. Creating many new arrays, running a bunch of extra function calls, creating extra closures, and really obfuscating code from the engine. This will tank performance.

This is the point at which people come back at me with something about "premature optimization" being bad. That's all well and good, but if you prematurely pessimize and let these patterns creep throughout your codebase, you end up with products that are significantly slower than they should be.

I've spent quite a while working on JS engines, and it always impresses me how much extra work exists in JS developers' code, seemingly for no real reason, and it's slowing down the entire internet. This doesn't appear to be better for the developer, the user, or any potential future maintainers.

Re: Self-Documenting Code

#104

Post by sesteel »

Looking at this thread, it is a wonder that any PRs make it through review. I started calling these kinds of debates Holographic Problems.

- Spaces vs Tabs

- Self documenting code vs documented code

- Error Codes vs Exceptions

- Monolithic vs Microservices Architectures

- etc.

Context matters and your context should probably drive your decisions, not your personal ideology. In other words, be the real kind of agile; stay flexible and change what needs to be changed as newly found information dictates.

Re: Self-Documenting Code

#105

Post by jonathanlydall »

fenomas wrote: post #92
jonathanlydall wrote: post #38

Earlier quoted context omitted.

I would say (as all good technical people know) it depends. I have come to appreciate the style of early returns rather than else statements as I have found over the years it generally makes the code easier for me to follow when I’m looking at it possibly years later. It really depends on the particular condition, but sometimes it just reads better to me to not use the else, and this is because as a style I tend to t…

It definitely depends, but personally I find early returns to be a bit of an antipattern IF they're based on business logic. If a function has lots of ifs, you can glance at the nesting to see which ones affect the line you want to edit, and ignore the others. But if the function has lots of returns, you have to check every one before a given line in order to know what constraints are true at that point. OTOH early r…

Calling it an anti pattern (as opposed to a subjective preference) is in my opinion super dangerous and reeks of cargo-culting as it implies an active avoidance of it which can result in deep nesting or contorted and hard to read logic.

There is no hard rule about early returns being always good or always bad, it depends on the particular situation.

Re: Self-Documenting Code

#106

Post by trealira »

38 wrote: post #52
lolinder wrote: post #50

Earlier quoted context omitted.

Then let's talk—with specifics!—about why it's a footgun and we shouldn't use it. "Because Go doesn't support it" would also be a reason to avoid: * Generics (at least until recently) * Classes * Prototypes * Exceptions * Async/Await * Package managers (until recently) * Algebraic data types * Effect types * Type inference * Type classes * Etc. You could argue that one or more of these are footguns, but I seriously d…

This isn't about Go. This is about a language construct (&& and ||) that I would argue is terrible and should never be used. Its mainly just sugar, and in my experience code heavy on these is harder to read and write. Couple that with truthiness and it's even worse.

> This is about a language construct (&& and ||) that I would argue is terrible and should never be used.

Something tells me you'd hate my five-line implementation of the standard library function fgets in C.

  char *fgets(char *buf, size_t n, FILE *fp) {
      char *p = buf;
      int c = 0;
      while (n > 1 && (c = getc(fp)) != EOF && (*p++ = c) != '\n')
          n--;
      return n > 0 && (c != EOF || feof(fp) && p != buf) ? *p = '\0', buf : 0;
  }
I'm joking; I'd never write code this way in earnest.

Re: Self-Documenting Code

#107

Post by kazinator »

jonathanlydall wrote: post #38
mega_dean wrote: post #27

Earlier quoted context omitted.

After that step, they say "The resulting code is shorter and has no nested logic." The resulting code has the same logic as before, it's just not visually represented as being nested. I've seen the same argument ("nesting is bad so indentation is a code smell") used to say that it's better to use early returns and omit the `else` block, eg: if (some_condition) { // do stuff here return; } // do other stuff here is "b…

I would say (as all good technical people know) it depends. I have come to appreciate the style of early returns rather than else statements as I have found over the years it generally makes the code easier for me to follow when I’m looking at it possibly years later. It really depends on the particular condition, but sometimes it just reads better to me to not use the else, and this is because as a style I tend to t…

Early returns are easy to read because although return is a staunchly imperative construct (a form of "go to"), early returns are structured such that they simulate a multi-case conditional from a functional language.

You know that each early return completely handles its respective case; if that branch is taken, that's it; the function has ended. There is only way to reach the code past the if/return, which is that the condition has to fail.

The conditionals inside a function that has a single return are harder to read, because no conditional is necessarily final.

if/elses that all return can be readable:

  if (this) {
    if (that) {
      return x;
    } else {
      return y;
    }
  } else {
    return z;
  }
still, it can be flattened:

  if (!this)
    return z;
  if (that)
    return x;
  return y;
It's shorter and less nested, and so that's a readability improvement. It's not as easy to see that x is returned when both this and that hold. The intermediate version below helps with that:

  if (this) {
    if (that)
      return x;
    return y;
  }

  return z;
If the conditions are cheaply tested variables, or simple expressions easily optimized by the compiler, there is also:

  if (this && that)
    return x;
  if (this)
    return y;
  return z;

Re: Self-Documenting Code

#108

Post by Mikhail_Edoshin »

Names do have some importance. If you pick random words and assign them to things you deal with you will find yourself unable to reason about them. Try it, it is interesting. Yet names are not the pinnacle of design. Far from it.

Look at a mechanical watch. (For example, here: https://ciechanow.ski/mechanical-watch/). Those little details, can you come up with self-documenting names for them? I do not think so. In programming good design is very much like that watch: it has lots of strangely-looking things that are of that shape because it fits their purpose [1]. There is no way to give them some presumably short labels that explain that purpose out of the context. Yet we need to point to them as we talk about them [2]. The role of names in programming is thus much more modest. In the order of importance:

- They must be distinct within the context (of course). - Yet their form must indicate the similarities between them: alen and blen are of the same kind and are distinct from abuf and bbuf, which are also of the same kind. - They must be pronounceable and reasonably short. Ideally they should be of the same length. - They need to have some semblance to the thing they represent. - It would be nice to make them consistent across different contexts. Yet this is is incredibly tedious task of exponential complexity.

There is also the overall notation. Ideally it should resemble written reasoning that follows some formal structure. None of existing notations is like that. The expressive tools in these notations are not meant to specify reasoning: they are meant to specify the work of a real or virtual machine of some kind. The fallacy of self-documenting code is an unrecognized desire to somehow reason with the knobs of that machine. It will not work this way. Yet a two-step process would work just fine: first you reason, then you implement this on the machine. But it will not look self-documenting, of course. P. S. This is a major problem in programming: we keep the code, but do not keep the reasoning that led to it.

[1] fitness for the purpose, Christopher Alexander, “The timeless way of building”. [2] notion vs definition, Evald Ilyenkov.

Re: Self-Documenting Code

#109

Post by kazinator »

fenomas wrote: post #92
jonathanlydall wrote: post #38

Earlier quoted context omitted.

I would say (as all good technical people know) it depends. I have come to appreciate the style of early returns rather than else statements as I have found over the years it generally makes the code easier for me to follow when I’m looking at it possibly years later. It really depends on the particular condition, but sometimes it just reads better to me to not use the else, and this is because as a style I tend to t…

It definitely depends, but personally I find early returns to be a bit of an antipattern IF they're based on business logic. If a function has lots of ifs, you can glance at the nesting to see which ones affect the line you want to edit, and ignore the others. But if the function has lots of returns, you have to check every one before a given line in order to know what constraints are true at that point. OTOH early r…

> you can glance at the nesting to see which ones affect the line you want to edit, and ignore the others.

Only if all the cases return! Only then is it obvious that you have independent cases. E.g. suppose we have three Boolean inputs x, y, z and want to do something for each binary combination:

  if (x) {
    if (y) {
      if (z) {
         return 7;
      } else {
         return 6;
      }
    } else { // x && !y
      if (z) {
         return 5;
      } else {
         return 4;
      }
    }
  } else { // !x
    if (y) { // !x && y
      if (z) {
         return 3;
      } else {
         return 2;
      }
    } else { // !x && !y
      if (z) {
         return 1;
      } else {
         return 0;
      }
    }    
  }
How would this look with early returns? One obvious way:

  if (x && y && z)
    return 7;
  if (x && y && !z)
    return 6;
  if (x && !y && z)
    return 5;
  // ... etc
(Let's ignore that we can just calculate the output with some bit twiddling; that's not the point).

Early return can be very clear, if we can bear repeatedly testing some conditions.

Re: Self-Documenting Code

#110

Post by Chris_Newton »

jcparkyn wrote: post #102
Chris_Newton wrote: post #86

If I were reviewing the original code, the first thing I’d question is the line user.password = await hashPassword(user.password); 1. As a rule, mutations are harder to understand than giving new names to newly defined values. 2. The mutation here apparently modifies an object passed into the function, which is a side effect that callers might not expect after the function returns. 3. The mutation here apparently cha…

Agreed, and then there's the time of check/time of use issue with creating a user. Probably not a vulnerability if userService is designed well, but still a bit dubious.

You’re right, that’s potentially a correctness issue as well. Ideally we’d have a creation interface that would also perform the pre-existence check atomically, so there would be no need for the separate check in advance and the potential race condition would not exist. This does depend on the user service providing a convenient interface like that, though, and alas we aren’t always that lucky.
Post reply on HN