Skip to content

Instantly share code, notes, and snippets.

@sethwklein
Last active May 18, 2021 18:06
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save sethwklein/5eba783bed06028d979e4307fda8ad24 to your computer and use it in GitHub Desktop.
Save sethwklein/5eba783bed06028d979e4307fda8ad24 to your computer and use it in GitHub Desktop.
Two types of parameters

Let's say you need to find the maximum number in a bunch of places, so you write a max method:

public static float Max(float a, float b) {
    if (a < b) {
        float t = a;
        a = b;
        b = t;
    }
    return a;
}

That might not be the simplest implementation ever, but sometimes the simplest thing is the second one that occurs to you, not the first. Also, that implementation makes the next examples work!

Then you need the minimum number in a bunch of places, so you extend your max method:

public static float Extreme(float a, float b, bool min = false) {
    if (a > b) {
        float t = a;
        a = b;
        b = t;
    }
    return min ? a : b;
}

That seems pretty sensible, until you compare it with the usability of the standard Min and Max methods.

Technically, Min, Max, and Extreme all take parameters, but the last parameter to Extreme is subtly different.

It's harder to use than separate methods because you have to think about more concepts when calling it, and it's harder to maintain because you have to think about more at once in order to modify it.

When you have these sort of switching parameters, a better choice is to make separate methods and pull out code they both need into private methods.

Here is example code demonstrating that. Obviously, the code in this example is worse because C# is too clumsy for composition on a problem this trivial. Fortunately, real world situations are complex enough that better code can result.

private struct Pair {
    public float a;
    public float b;
}

private static Pair Swap(Pair p) {
    float t = p.a;
    p.a = p.b;
    p.b = t;
    return p;
}

public static float Min(float a, float b) {
    Pair p = new Pair {
        a = a,
        b = b,
    };
    if (a > b) {
        p = Swap(p);
    }
    return p.a;
}

public static float Max(float a, float b) {
    Pair p = new Pair {
        a = a,
        b = b,
    };
    if (a < b) {
        p = Swap(p);
    }
    return p.a;
}
@willemOH
Copy link

Ok, I can see the potential here for easier to call methods. My understanding is that shifting priorities from concise code to easy to use code is the main take away here. Would that be accurate?

@sethwklein
Copy link
Author

Yes, complected is worse than verbose. But!

Concision still matters, and in real situations, I'd expect the composition based implementation to be not much more verbose.

Also, the final example is a terrible implementation of Min and Max, and the internal lack of concision is part of what's wrong with it. Given that swap is three trivial and well known lines that won't change unless C# changes, there's little justification for putting it in a method when doing so is this clumsy. But what's worse is that the entire swap concept is unneeded. You can implement Min and Max with only return, less than, and if.

@sethwklein
Copy link
Author

Technically, Min, Max, and Extreme all take parameters, but the last parameter to Extreme is subtly different.

That difference is what I'm trying to illustrate.

@semateos
Copy link

What I used to do when I wrote code like that was if the mutli-purpose function was easier to maintain once instead of twice, do that, and ALSO write the easier to use and understand interfaces to it. So like you'd have the Extreme function, and then write simple Min and Max functions that use it. The decision point there is around how useful it is to have and maintain the more complex mutli-purpose function. Those little functions can also serve as a kind of documentation for how to use the more complex one.

@justinomalley
Copy link

justinomalley commented May 11, 2021

I definitely agree that the explicitly defined Min and Max functions are easier to use, and maintain and all that. Here's an example of what I would actually implement based on this advice:

public static float Min(float a, float b) {
    return a < b ? a : b;
}

public static float Max(float a, float b) {
    return a > b ? a : b;
}

which eliminates the need for any separate private method or struct. I assume the complexity in that last example and the Pair/Swap concept were just included to illustrate how functionality shared between some methods can be separated out into a shared private method. I do agree with that general idea applied to more complicated problems; I can try to avoid these types of switching parameters when defining public interfaces for sure.

@sethwklein
Copy link
Author

sethwklein commented May 11, 2021

I assume the complexity in that last example and the Pair/Swap concept were just included to illustrate how functionality shared between some methods can be separated out into a shared private method.

Yep! In the code you wrote, @justinomalley, </>, the ternary operator, and return are effectively "methods" from which you composed Min and Max, but you didn't have to write them and they have different interfaces. The whole business starts to make sense once you get up a couple levels, not just one level, from what the programming language supplies.

@sethwklein
Copy link
Author

@semateos, I see that in the Go standard library frequently. I'd probably compose more than they do, but as you say, it's a judgement call.

@wvanooijen92
Copy link

@sethwklein Conceptually I agree! In the case of Min / Max, I'd go for built-in methods.

https://golang.org/pkg/math/#Min
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/min

is that a bad thing?

Also, sometimes (I think when writing complex business logic), it may become a little bit difficult to see when something becomes "multi-purpose". Let's say you are writing logic to produce a profit-and-loss report. Based on your data, you will have to go left or right at some point in the decision tree, e.g. a private car owner pays different taxes than a car that has a business lease.

Check out these two examples;

private function doGenericStuff(){
     return Database.fetchA()
}
private function getPrivateCarStuff(){
    return Database.fetchB1()
}
private function getBusinessCarStuff(){
   return Database.fetchB2()
}
public function getPLReport(){
    const data = doGenericStuff()
    if(condition){
        data.carStuff= getPrivateCarStuff();
    } else {
        data.carStuff= getBusinessCarStuff();
    }
    return data;
}
private function doGenericStuff(){
     return Database.fetchA()
}

private function getCarStuff(){
    let data;
    if(condition){
        data = Database.fetchB1();
    } else {
        data = Database.fetchB2();
    }
    return data
}

public function getPLReport(){
    const data = doGenericStuff()
    data.carStuff = getCarStuff()
    return data;
}

I am not sure which one is better, this just came to mind.

@sethwklein
Copy link
Author

@wvanooijen92, I never dreamed that you'd think I was implying one should reimplement min and max!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment