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

Exhaustive AND have a default? #24

Open
mikecann opened this issue Dec 9, 2021 · 4 comments
Open

Exhaustive AND have a default? #24

mikecann opened this issue Dec 9, 2021 · 4 comments
Labels
good first issue Good for newcomers

Comments

@mikecann
Copy link

mikecann commented Dec 9, 2021

I am still loving Variant and using it on a daily basis so MASSIVE thanks for this awesome lib :)

Up to now Variant has done just about everything I wanted. I have however started to run into a scenario that is causing problems. Its best described with an example.

e.g. I have the following

interface Attack {
  kind: `attack`;
  playerId: string;
}

interface Timeout {
  kind: `timeout`;
  playerId: string;
}

interface Surrender {
  kind: `surrender`;
  playerId: string;
}

export type BattleFinishedReason = Attack | Timeout | Surrender;

export const calculateIfBattleWasAbandoned = (reason: BattleFinishedReason ): boolean => 
  matchKind(reason, {
    timeout: () => true,
    surrender: () => true,
    attack: () => false,
  });

This is great, I release this code as v1. The server and client are working great.

I now do a v2 of the game and add a new type to the union..

...

interface Cancel {
  kind: `cancel`;
  playerId: string;
}

export type BattleFinishedReason = Attack | Timeout | Surrender | Cancel;

The compiler will now yell at me that there is a missing option in the calculateIfBattleWasAbandoned match which is great, so I add that:

export const calculateIfBattleWasAbandoned = (reason: BattleFinishedReason ): boolean => 
  matchKind(reason, {
    timeout: () => true,
    surrender: () => true,
    attack: () => false,
    cancel: () => true,
  });

I then push the code as a v2 release. The server is happy because it can handle the new case but the problem is there are clients still on v1 of the game and so their calculateIfBattleWasAbandoned matcher doesnt know how to handle the Cancel BattleFinishedReason ..

I know this can be solved with a default option in the matcher:

export const calculateIfBattleWasAbandoned = (reason: BattleFinishedReason ): boolean => 
  matchKind(reason, {
    timeout: () => true,
    surrender: () => true,
    attack: () => false,
    cancel: () => true,

    // Saving future me some headaches
    default: () => false
  });

... but this breaks the exhustive checking which I rely on so much.

So is it possible for the matcher to be both exhaustive AND have a default?

@paarthenon
Copy link
Owner

Hello!

This is an interesting situation where are your type-time expectations differ from your runtime truth. I think it would be appropriate to add a guard in front of the match, accounting for this unanticipated widening of the possibilities.

const calculateIfBattleWasAbandoned = (reason: BattleFinishedReason): boolean => {
  if (isOfVariant(reason, BattleFinishedReason)) {
    return matchKind(reason, {
      timeout: () => true,
      surrender: () => true,
      attack: () => false,
      cancel: () => true,
    });
  } else {
    // Saving future me some headaches
    // log the edge case
    return false;
  }
}

If BattleFinishedReason is a variant, then the above call to isOfVariant works well. If not, you can write your own check. It could be as simple as

const validTypes = [
  `attack`,
  `cancel`,
  `surrender`,
  `timeout`,
]

const calculateIfBattleWasAbandoned = (reason: BattleFinishedReason): boolean => {
  if (validTypes.includes(reason.type)) {
    return matchKind(reason, {
      timeout: () => true,
      surrender: () => true,
      attack: () => false,
      cancel: () => true,
    });
  } else {
    // Saving future me some headaches
    // log the edge case
    return false;
  }
}

This gives an error checking flow outside the match allowing us to handle the unexpected cases, while also benefiting from exhaustiveness for when things do work out our way. How does that strike you?

@mikecann
Copy link
Author

mikecann commented Dec 9, 2021

Yes this is effectively the solution I came ypu with too..

export const calculateIfBattleWasAbandoned = (reason: BattleFinishedReason ): boolean => 
  matchKind(reason, {
    timeout: () => true,
    surrender: () => true,
    attack: () => false,   
  }) ?? false

I thought I would bring up up incase there was a better way tho.

Thanks :)

Feel free to close.

@paarthenon
Copy link
Owner

It's been a while, but I never closed this because I intend to implement this fallback behavior. It makes sense, the default case handling through ?? is actually what I do too but it's a bit unintuitive. Thanks for raising the issue.

Match will get a helper function (like partial) and matcher will get another function that achieves the same functionality.

@paarthenon paarthenon added the good first issue Good for newcomers label Oct 4, 2022
@paarthenon
Copy link
Owner

Hello, I have a new update that should address this issue. Please let me know if I've missed any of the desired functionality. These changes are available in 3.0.0-dev.25

I've added a withFallback helper. It takes two parameters, a completed handler object and a fallback function. As these cases are exceptional, I left the fallback function's input as unknown.

export const calculateIfBattleWasAbandoned = (reason: BattleFinishedReason): boolean =>
    match(reason, withFallback({
        timeout: () => true,
        surrender: () => true,
        attack: () => false,
    }, () => false))

For matcher() rather than adding it as a new terminal, I expanded .complete() to include this fallback functionality, since I expect users would still want exhaustiveness. Here's an example of that in use:

export const calculateIfBattleWasAbandoned = (reason: BattleFinishedReason): boolean =>
    matcher(reason)
        .when(['timeout', 'surrender'], () => true)
        .when('attack', () => false)
        .complete({
            withFallback: () => false
        })

On the redux side, my rootReducer now looks like this:

export const rootReducer = (state = initState, action: Action) => {
    return matcher(action)
        .when(types(AppAction), _ => ...)
        .when(types(GameAction), _ => ...)
        .complete({
            withFallback: _ => state, // pass through redux init and persist actions
        })
};

which I would call an upgrade. I hope you feel the same.

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

No branches or pull requests

2 participants