20

I often find myself writing functions that look like this because they allow me to easily mock data access, and still provide a signature that accepts parameters to determine what data to access.

public static string GetFormattedRate(
        Func<string, RateType>> getRate,
        string rateKey)
{
    var rate = getRate(rateKey);
    var formattedRate = rate.DollarsPerMonth.ToString("C0");
    return formattedRate;
}

Or

public static string GetFormattedRate(
        Func<RateType, string> formatRate,
        Func<string, RateType>> getRate,
        string rateKey)
{
    var rate = getRate(rateKey);
    var formattedRate = formatRate(rate);
    return formattedRate;
}

Then I use it something like this:

using FormatterModule;

public static Main()
{
    var getRate = GetRateFunc(connectionStr);
    var formattedRate = GetFormattedRate(getRate, rateType);
    // or alternatively
    var formattedRate = GetFormattedRate(getRate, FormatterModule.FormatRate, rateKey);

    System.PrintLn(formattedRate);
}

Is this a common practice? I feel like I should be doing something more like

public static string GetFormattedRate(
        Func<RateType> getRate())
{
    var rate = getRate();
    return rate.DollarsPerMonth.ToString("C0");
}

But that doesn't seem to work very well because I'd have to make a new function to pass into the method for every rate type.

Sometimes I feel like I should be doing

public static string GetFormattedRate(RateType rate)
{
   return rate.DollarsPerMonth.ToString("C0");
}

But that seems to take away any fetch and format re-usability. Whenever I want to fetch and format I have to write two lines, one to fetch and one to format.

What am I missing about functional programming? Is this the right way to do it, or is there a better pattern that's both easy to maintain and use?

  • 50
    The DI cancer has spread so far... – Idan Arye Mar 20 '17 at 23:30
  • 16
    I struggle to see why this structure would be used in the first place. Surely it's more convenient (and clear) for GetFormattedRate() to accept the rate to format as a parameter, as opposed to having it accept a function that returns the rate to format as a parameter? – aroth Mar 21 '17 at 2:25
  • 6
    A better way is making use of closures where you pass the parameter itself to a function, which in return gives you a function referring to that specical parameter. This "configured" function would be passed as a parameter to the function, which uses it. – Thomas Junk Mar 21 '17 at 7:49
  • 7
    @IdanArye DI cancer? – Jules Mar 21 '17 at 13:26
  • 11
    @Jules dependency injection cancer – cat Mar 21 '17 at 16:20
40

If you do this long enough, you'll eventually find yourself writing this function over and over:

public static Type3 CombineFunc1AndFunc2(
    Func<Type1, Type2> func1,
    Func<Type2, Type3>> func2,
    Type1 input)
{
    return func2(func1(input))
}

Congratulations, you've invented function composition.

Wrapper functions like this don't have much use when they're specialized to one type. However, if you introduce some type variables and omit the input parameter, then your GetFormattedRate definition looks like this:

public static Func<A, C> Compose(
    Func<B, C> outer, Func<A, B>> inner)
{
    return (input) => outer(inner(input))
}

var GetFormattedRate = Compose(FormatRate, GetRate);
var formattedRate = GetFormattedRate(rateKey);

As it stands, what you're doing has little purpose. It's not generic, so you need to duplicate that code all over the place. It overcomplicates your code because now your code has to assemble everything it needs from a thousand tiny functions on its own. Your heart's in the right place though: you just need to get used to using these sorts of generic higher order functions to put things together. Or, use a good old fashion lambda to turn Func<A, B> and A into Func<B>.

Don't repeat yourself.

  • 16
    Do repeat yourself if avoiding repeating yourself makes the code worse. Such as if you always write those two lines instead of FormatRate(GetRate(rateKey)). – immibis Mar 21 '17 at 8:24
  • 6
    @immibis I guess the idea is that he'll be able to use GetFormattedRate directly from now on. – Carles Mar 21 '17 at 13:41
  • I think this is what I'm trying to do here, and I've tried this Compose function before but it seems that I rarely get to use it because my 2nd function often needs more than one parameter. Perhaps I need to do this in combination with closures for configured functions as mentioned by @thomas-junk – rushinge Mar 21 '17 at 19:44
  • @rushinge This kind of composition works on the typical FP function which always has a single argument (additional arguments are really functions of their own, think of it like Func<Func<A, B>, C>); this means that you only need one Compose function that works for any function. However, you can work with C# functions well enough just using closures - instead of passing Func<rateKey, rateType>, you onlry really need Func<rateType>, and when passing the func, you build it like () => GetRate(rateKey). The point is that you don't expose the arguments the target function doesn't care about. – Luaan Mar 22 '17 at 13:38
  • 1
    @immibis Yes, the Compose function is really only useful if you need to delay the execution of GetRate for some reason, such as if you want to pass Compose(FormatRate, GetRate) to a function which provides a rate of its own choosing, e.g. to apply it to every element in a list. – jpaugh Mar 22 '17 at 14:01
108

There is absolutely no reason to pass a function, and its parameters, only to then call it with those parameters. In fact, in your case you have no reason to pass a function at all. The caller might as well just call the function itself and pass the result.

Think about it - instead of using:

var formattedRate = GetFormattedRate(getRate, rateType);

why not simply use:

var formattedRate = GetFormattedRate(getRate(rateType));

?

As well as reducing unnecessary code it also reduces coupling - if you want to change how the rate is fetched (say, if getRate now needs two arguments) you don't have to change GetFormattedRate.

Likewise, there's no reason to write GetFormattedRate(formatRate, getRate, rateKey) instead of writing formatRate(getRate(rateKey)).

Don't overcomplicate things.

  • 3
    In this case you are correct. But if the inner function were called multiple times, say in a loop or a map function, then the ability to pass in arguments would be useful. Or use functional composition / currying as proposed in @Jack answer. – user949300 Mar 21 '17 at 2:06
  • 15
    @user949300 maybe, but that's not the OP's use case (and if it was, perhaps it's formatRate that should be mapped over the rates that should be formatted). – jonrsharpe Mar 21 '17 at 3:23
  • 4
    @user949300 Only if your language does not support closures or when lamdas are a hazzle to type out – Bergi Mar 21 '17 at 11:59
  • 4
    Note that passing a function and its parameters to another function is a totally valid way to delay evaluation in a language without lazy semantics en.wikipedia.org/wiki/Thunk – Jared Smith Mar 21 '17 at 13:49
  • 4
    @JaredSmith Passing the function, yes, passing its parameters, only if your language doesn't support closures. – immibis Mar 21 '17 at 20:50
15

If you absolutely need to pass a function into the function because it passes some extra argument or calls it in a loop then you can instead pass a lambda:

public static string GetFormattedRate(
        Func<string> getRate)
{
    var rate = getRate();
    var formattedRate = rate.DollarsPerMonth.ToString("C0");
    return formattedRate;
}

var formattedRate = GetFormattedRate(()=>getRate(rateKey));

The lambda will bind the arguments the function doesn't know about and hide that they even exist.

-1

Isn't this what you want?

class RateFormatter
{
    public abstract RateType GetRate(string rateKey);

    public abstract string FormatRate(RateType rate);

    public string GetFormattedRate(string rateKey)
    {
        var rate = GetRate(rateKey);
        var formattedRate = FormatRate(rate);
        return formattedRate;
    }
}

And then call it like this:

static class Program
{
    public static void Main()
    {
        var rateFormatter = new StandardRateFormatter(connectionStr);
        var formattedRate = rateFormatter.GetFormattedRate(rateKey);

        System.PrintLn(formattedRate);
    }
}

If you want a method that can behave in multiple different ways in an object-oriented language such as C#, the usual way to do that is to have the method call an abstract method. If you don't have a specific reason to do it a different way, you should do it that way.

Does this look like a good solution, or are there downsides you're thinking of?

  • 1
    There are a couple of wonky things in your answer (why is the formatter also getting the rate, if it's just a formatter? You could also remove the GetFormattedRate method and just call IRateFormatter.FormatRate(rate)). The basic concept however is correct, and I think too OP should implement its code polymorphicaly if he needs multiple formatting methods. – BgrWorker Mar 22 '17 at 8:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.