# Best convention/style for pattern matching

#1

Hello fellow Reason friends!

I am trying to write an expression like this into reason:

``````function priceTierCalc(price) {
if (price > 0 && price < 50) {
return 0.33 * price;
} else if (price > 51 && price < 100) {
return 0.29 * price;
} else (price > 101 && price < 200) {
return 0.26 * price;
}
}

``````

And I have written something like this:

``````let priceTierCalc = (price) => {
switch(price) {
| price when price > 0 && price < 50 => 0.33 *. float_of_int(price)
| price when price > 51 && price < 100 => 0.29 *. float_of_int(price)
| price when price > 101 && price < 200 => 0.26 *. float_of_int(price)
}
};

``````

Which seems to be working fine, but I am getting this message from the compiler:

``````bad style, all clauses in this pattern-matching are guarded.
``````

Please excuse my newbiness, but what is wrong with having all clauses guarded? If there is a better way to write the expression above, could someone enlighten me?

Thanks!

#2

Iâ€™d probably move all the conditions to a tuple:

``````let priceTierCalc = price =>
switch (
price > 0 && price < 50,
price > 51 && price < 100,
price > 101 && price < 200,
) {
| (true, _, _) => 0.33 *. float_of_int(price)
| (_, true, _) => 0.29 *. float_of_int(price)
| (_, _, true) => 0.26 *. float_of_int(price)
};
``````

Which has the benefit that now bsb will tell you:

``````Warning 8: this pattern-matching is not exhaustive.
Here is an example of a value that is not matched:
(false, false, false)
``````

(which I think is a problem in both of your examples, but bsb couldnâ€™t tell in the reason version because of the guards)

Alternatively, if these are known tiers, I might make a variant that would return return e.g. `HighPriceTier(discount)` and switch over that, but that may not match your domain well.

#3

Mmmâ€¦I see! Because the conditions are guarded the compiler canâ€™t tell if all cases are covered.

In my example I only included 3 tiers, but the function I am writing actually has 9 tiers. Would you still move all your condition to a tuple when thereâ€™s nine? That could look pretty verbose, no?

Also what you said about using variants sounds interesting:

This isnâ€™t semantically correct, but would it look something like this?

``````type = Discount | HigherDiscount | HighestDiscount;

switch (price) {
| Discount  when price > 0 && price < 50 => 0.33 * float_of_int(price)
| HigherDiscount  when price > 51 && price < 100 => 0.29 * float_of_int(price)
| HighestDiscount  when price > 101 && price < 200 => 0.26 * float_of_int(price)
};
``````

I guess I donâ€™t quite understandâ€¦can you expound on how to switch over constructors in this case?

#4

As @sgrove mentioned, the guard clauses did not match all possible inputs (which I assume is why itâ€™s a bad style). You can call the function with a -1 and itâ€™ll throw an match error at runtime. There was an assumption based on the domain, that no one will have a negative price.

Anywayâ€¦ back to the codeâ€¦ you can add a catch all case to throw an exception when a negative number is entered or a price over 200 in your example above. Generally I always avoid the `_` catch all clause unless itâ€™s going to throw because itâ€™s a source of a lot of bugs (for example defaulting to a certain plan and then forgetting about it 2 years later when you modify something else). Iâ€™ve also added the suffix `Exn` to the end to indicate to future maintainers that this function will throw a runtime exception if the data does not match the expectation.

``````let priceTierCalcExn = price => {
let base =
switch (price) {
| price when price > 0 && price < 50 => 0.33
| price when price > 51 && price < 100 => 0.29
| price when price > 101 && price < 200 => 0.26
| _else => raise(Not_found)
};
base * float_of_int(price);
};
``````

I agree with @sgrove on the variantsâ€¦ In an ideal situation you can use the compiler for more than just null checks and to make sure you pass in a string instead of an int. You can embed business logic in the type system to make sure that when you refer to a `PricePlan22` it will easier to pattern match against in all of your program (and can likely be changed in one spot).

Iâ€™ve came up with a rough example below. You would most likely use a plan ID from your database or Stripe plan ID and run that through a parser to return the subscription variants. Instead of hard coding you could also use ENV variables but then youâ€™ll lose some type safety as the program has to assume theyâ€™re present (unless you check on startup I guess).

The purpose here is that you can now pass around `Plan1`, `Plan2` and if you change anything related to the plans it can show you helpful errors. It also removes the need to have a raise branch in the switch since it will not compile if the variants donâ€™t match.

My guess is that the most useful thing about this change would be the case of adding a new variant to `Subscription.plan`. Then youâ€™ll see a bunch of warnings/errors about a non exhaustive match where youâ€™ve forgotten a caseâ€¦ then you can manage the logic/handling for this new plan in the areas sprinkled throughout the app. Ideally you want the exhaustive match warning option changed to a compile error to keep things sound since you can still pass CI and deploy with warnings.

Unrelated to Reason, but I would also stay away from making variants like `GoldPlan, SilverPlan, BronzePlan`, because youâ€™re going to end up changing the gold plan to have different features but you donâ€™t want to change existing customers. `Plan1` and `Plan22` may both be gold but `Plan22` may include features that the early adopters (`Plan1`) currently have.

``````/* Subscription.re */
module Subscription = {
type plan =
| Plan1
| Plan2
| Plan3;

let parsePlanFromStripeIdExn = (id) =>
switch (id) {
| "j39xd" => Plan1
| "khx3d" => Plan2
| "js7fn" => Plan3
| _ => raise(Not_found)
};
};

/* User.re */
module User = {
type t = {
id: int,
name: string,
plan: Subscription.plan,
};
};

/* when you parse user data into a user record, choose plan */
let currentUser : User.t = {id: 22, name: "Jane", plan: Subscription.Plan2};

let priceTierCalc = (price, plan: Subscription.plan) => {
let base =
switch (plan) {
| Plan1 => 0.33
| Plan2 => 0.29
| Plan3 => 0.26
};
base *. float_of_int(price);
};

Js.log(
priceTierCalc(1, currentUser.plan)
);
``````

#5

Hey @SkinnyGeek1010 !
Thatâ€™s great to know that the special fall-through case from your experience has been a source of a lot of bugsâ€¦I will use it sparingly.
And thank you for your example and explanation on what the logic might look like using variantsâ€”itâ€™s very helpful!! I wasnâ€™t quite sure how to express that, but this really helps.

#6

Since itâ€™s my purpose in life to ask silly questions: Wouldnâ€™t code like this â€“

``````    switch (price) {
| price when price > 0 && price < 50 => 0.33
| price when price > 51 && price < 100 => 0.29
``````

be considered as not covering all cases because a price of 50 is not less than 50 nor greater than 51, so it matches neither the first nor second condition?

#7

@jdeisenberg haha yes you are right. Thanks for pointing that out!

#8

Thisâ€¦ this is one of those use cases where property based testing is soooo helpful Good catch!

#9

Hello

Just came here to tell what I would probably do in that scenario. If thereâ€™s nothing else to check on conditions besides price range I would just have a list with the values and find the one that matches the price.

``````let priceTierCalc = price => {
let tiers = [
(0, 50, 0.33),
(50, 100, 0.29),
(50, 200, 0.26)
];

let (_, _, discount) =
List.find(((l, h, _)) => price > l && price <= h, tiers);

discount *. float_of_int(price);
};
``````

Bonus points, `List.find` already throws Not_found if no match is found.
I like this better, because adding/updating tiers is now easy.

#10

I love seeing different approaches! Thank you for sharing!!