Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should intoTuple use undefined instead of null? #14

Closed
traverse1984 opened this issue Oct 28, 2022 · 5 comments
Closed

Should intoTuple use undefined instead of null? #14

traverse1984 opened this issue Oct 28, 2022 · 5 comments

Comments

@traverse1984
Copy link
Owner

From this comment: #11 (comment).

On the one hand, this pattern looks nice and simplifies the case where you want to introduce default values. On the other hand, given that it may sometimes be necessary to be more explicit to correctly narrow the types, it would require somewhat unsightly uses of if (err === undefined) or using ==, neither of which I am a huge fan of.

@thelinuxlich
Copy link

But undefined returns true for the same truthy comparisons of null

@thelinuxlich
Copy link

I have a helper for this style of error handling, similar to tiny-invariant:

function throwIf<T>(
  err: T | undefined,
  message?: string
): asserts err is undefined {
  if (err !== undefined) throw new Error(message ?? err?.toString())
}

// then on common usage:
const [err, res] = await generateResult().intoTuple()
throwIf(err) // if err is undefined res gets narrowed to T instead of T | undefined
doSomething(res)

@traverse1984
Copy link
Owner Author

traverse1984 commented Oct 30, 2022

But undefined returns true for the same truthy comparisons of null

Yeah - I think I'm just suffering from visual issues. I don't like how x === undefined looks and I don't want to encourage truthy comparisons, not least because I'm so used to writing === that I'm sure to mess that up on a number of occasions. However, I'm willing to concede this might be (probably is) a non-issue.

I have a helper for this style of error handling, similar to tiny-invariant:

I think your helper and that pattern could indicate the need for a better expect API - eg:

class Result<T, E> {
   // ...
   expect(this: Result<T, E>, msg: string): T;
   expect<E extends Error>(this: Result<T, E>, msg?: string): T;
   expect(this: Result<T, E>, msg?: string): T {
      if (this[T]) {
         return this[Val] as T;
      } else if (msg === undefined) {
         throw this[Val];
      } else {
         throw new Error(msg);
      }
   }
}

const res = (await generateResult()).expect();
doSomething(res);

@thelinuxlich
Copy link

You gave me a good idea, but this would be a optional behavior because sometimes you want to do something else instead of throwing the err

@traverse1984
Copy link
Owner Author

Will implement this in next release, see #15.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants