38

We have an API function that breaks down a total amount into monthly amounts based on given start and end dates.

// JavaScript

function convertToMonths(timePeriod) {
  // ... returns the given time period converted to months
}

function getPaymentBreakdown(total, startDate, endDate) {
  const numMonths = convertToMonths(endDate - startDate);

  return {
    numMonths,
    monthlyPayment: total / numMonths,
  };
}

Recently, a consumer for this API wanted to specify the date range in other ways: 1) by providing the number of months instead of the end date, or 2) by providing the monthly payment and calculating the end date. In response to this, the API team changed the function to the following:

// JavaScript

function addMonths(date, numMonths) {
  // ... returns a new date numMonths after date
}

function getPaymentBreakdown(
  total,
  startDate,
  endDate /* optional */,
  numMonths /* optional */,
  monthlyPayment /* optional */,
) {
  let innerNumMonths;

  if (monthlyPayment) {
    innerNumMonths = total / monthlyPayment;
  } else if (numMonths) {
    innerNumMonths = numMonths;
  } else {
    innerNumMonths = convertToMonths(endDate - startDate);
  }

  return {
    numMonths: innerNumMonths,
    monthlyPayment: total / innerNumMonths,
    endDate: addMonths(startDate, innerNumMonths),
  };
}

I feel this change complicates the API. Now the caller needs to worry about the heuristics hidden with the function's implementation in determining which parameters take preference in being used to calculate the date range (i.e. by order of priority monthlyPayment, numMonths, endDate). If a caller doesn't pay attention to the function signature, they might send multiple of the optional parameters and get confused as to why endDate is being ignored. We do specify this behavior in the function documentation.

Additionally I feel it sets a bad precedent and adds responsibilities to the API that it should not concern itself with (i.e. violating SRP). Suppose additional consumers want the function to support more use cases, such as calculating total from the numMonths and monthlyPayment parameters. This function will become more and more complicated over time.

My preference is to keep the function as it was and instead require the caller to calculate endDate themselves. However, I may be wrong and was wondering if the changes they made were an acceptable way to design an API function.

Alternatively, is there a common pattern for handling scenarios like this? We could provide additional higher-order functions in our API that wrap the original function, but this bloats the API. Maybe we could add an additional flag parameter specifying which approach to use inside of the function.

6
  • 79
    "Recently, a consumer for this API wanted to [provide] the number of months instead of the end date" - This is a frivolous request. They can transform the # of months into a proper end date in a line or two of code on their end.
    – GHP
    Commented Sep 24, 2019 at 20:50
  • 12
    that looks like a Flag Argument anti-pattern, and I would also recommend splitting into several functions
    – njzk2
    Commented Sep 25, 2019 at 6:14
  • 2
    As a side note, there are functions that can accept the same type and number of parameters and produce very different results based on those - see Date - you can supply a string and it can be parsed to determine the date. However, this way oh handling parameters can also be very finicky and might produce unreliable results. See Date again. It's not impossible to do right - Moment handles it way better but it's very annoying to use regardless.
    – VLAZ
    Commented Sep 25, 2019 at 6:20
  • On a slight tangent, you might want to think about how to handle the case where monthlyPayment is given but total is not an integer multiple of it. And also how to deal with possible floating point roundoff errors if the values aren't guaranteed to be integers (e.g. try it with total = 0.3 and monthlyPayment = 0.1). Commented Sep 25, 2019 at 8:22
  • @Graham I didn't react to that... I reacted to the next statement "In response to this, the API team changed the function..." - rolls up into fetal position and starts rocking - It doesn't matter where that line or two of code goes, either a new API call with the different format, or done on the caller end. Just don't change a working API call like this!
    – Baldrickk
    Commented Sep 26, 2019 at 13:52

6 Answers 6

99

Seeing the implementation, it appears to me what you really require here is 3 different functions instead of one:

The original one:

function getPaymentBreakdown(total, startDate, endDate) 

The one providing the number of months instead of the end date:

function getPaymentBreakdownByNoOfMonths(total, startDate, noOfMonths) 

and the one providing the monthly payment and calculating the end date:

function getPaymentBreakdownByMonthlyPayment(total, startDate, monthlyPayment) 

Now, there are no optional parameters any more, and it should be pretty clear which function is called how and for which purpose. As mentioned in the comments, in a strictly typed language, one could also utilize function overloading, distinguishing the 3 different functions not necessarily by their name, but by their signature, in case this does not obfuscate their purpose.

Note the different functions don't mean you have to duplicate any logic - internally, if these functions share a common algorithm, it should be refactored to a "private" function.

is there a common pattern for handling scenarios like this

I don't think there is a "pattern" (in the sense of the GoF design patterns) which describes good API design. Using self-describing names, functions with fewer parameters, functions with orthogonal (=independent) parameters, are just basic principles of creating readable, maintainable and evolvable code. Not every good idea in programming is necessarily a "design pattern".

13
  • 24
    Actually the "common" implementation of the code could simply be getPaymentBreakdown (or really any one of those 3) and the other two functions just convert the arguments and call that. Why add a private function that is a perfect copy of one of these 3? Commented Sep 25, 2019 at 7:18
  • @GiacomoAlzetta: that is possible. But I am pretty sure the implementation will become simpler by providing a common function which contains only the "return" part of the OPs function, and let the public 3 functions call this function with parameters innerNumMonths, total and startDate. Why keep an overcomplicated function with 5 parameters, where 3 are almost optional (except one has to be set), when a 3-parameter function will do the job as well?
    – Doc Brown
    Commented Sep 25, 2019 at 7:31
  • 3
    I didn't mean to say "keep the 5 argument function". I'm just saying that when you have some common logic this logic need not be private. In this case all 3 functions can be refactored to simply tranform the parameters to start-end dates, so you can use the public getPaymentBreakdown(total, startDate, endDate) function as common implementation, the other tool will simply compute the suitable total/start/end dates and call it. Commented Sep 25, 2019 at 8:13
  • @GiacomoAlzetta: ok, was a misunderstanding, I thought you were talking about the second implementation of getPaymentBreakdown in the question.
    – Doc Brown
    Commented Sep 25, 2019 at 9:13
  • I'd go as far as adding a new version of the original method that's explicitly called 'getPaymentBreakdownByStartAndEnd' and deprecating the original method, if you want to supply all of these.
    – Erik
    Commented Sep 25, 2019 at 12:18
20

Additionally I feel it sets a bad precedent and adds responsibilities to the API that it should not concern itself with (i.e. violating SRP). Suppose additional consumers want the function to support more use cases, such as calculating total from the numMonths and monthlyPayment parameters. This function will become more and more complicated over time.

You're exactly correct.

My preference is to keep the function as it was and instead require the caller to calculate endDate themselves. However, I may be wrong and was wondering if the changes they made were an acceptable way to design an API function.

This isn't ideal either, because the caller code will be polluted with unrelated boiler plate.

Alternatively, is there a common pattern for handling scenarios like this?

Introduce a new type, like DateInterval. Add whatever constructors make sense (start date + end date, start date + num months, whatever.). Adopt this as the common-currency types for expressing intervals of dates/times throughout your system.

4
  • 3
    @DocBrown Yep. In such cases (Ruby, Python, JS), it's customary to just use static/class methods. But that's an implementation detail, that I don't think is particularly relevant to my answer's point ("use a type").
    – Alexander
    Commented Sep 24, 2019 at 21:37
  • 2
    And this idea sadly reaches its limits with the third requirement: Start Date, total Payment and a monthly Payment - and the function will calculate the DateInterval from the money parameters - and you should not put the monetary amounts into your date range...
    – Falco
    Commented Sep 25, 2019 at 7:44
  • 3
    @DocBrown "only shift the problem from the existing function to the constructor of the type" Yes, it's putting time code where time code should go, so that money code can be where money code should go. It's simple SRP, so I'm not sure what you're getting at when you say it "only" shifts the problem. That's what all functions do. They don't make code go away, they move it into more appropriate places. What's your issue with that? "but my congratulations, at least 5 upvoters took the bait" This sounds a lot more assholeish than I think (hope) you intended.
    – Alexander
    Commented Sep 25, 2019 at 14:39
  • @Falco That sounds like a new method to me (on this payment calculator class, not DateInterval): calculatePayPeriod(startData, totalPayment, monthlyPayment)
    – Alexander
    Commented Sep 25, 2019 at 14:42
7

Sometimes fluent-expressions help on this:

let payment1 = forTotalAmount(1234)
                  .breakIntoPayments()
                  .byPeriod(months(2));

let payment2 = forTotalAmount(1234)
                  .breakIntoPayments()
                  .byDateRange(saleStart, saleEnd);

let monthsDue = forTotalAmount(1234)
                  .calculatePeriod()
                  .withPaymentsOf(12.34)
                  .monthly();

Given enough time to design, you can come up with a solid API that acts similar to a domain-specific-language.

The other big advantage is that IDEs with autocomplete make almost irrevelant to read the API documentation, as is intuitive due its self-discoverable capabilities.

There are resources out there such as https://nikas.praninskas.com/javascript/2015/04/26/fluent-javascript/ or https://github.com/nikaspran/fluent.js on this topic.

Example (taken from the first resource link):

let insert = (value) => ({into: (array) => ({after: (afterValue) => {
  array.splice(array.indexOf(afterValue) + 1, 0, value);
  return array;
}})});

insert(2).into([1, 3]).after(1); //[1, 2, 3]
11
  • 8
    Fluent interface by itself doesn't make any particular task easier or harder. This seems more like the Builder pattern.
    – VLAZ
    Commented Sep 25, 2019 at 6:25
  • 8
    The implementation would be rather complicated though if you need to prevent mistaken calls like forTotalAmount(1234).breakIntoPayments().byPeriod(2).monthly().withPaymentsOf(12.34).byDateRange(saleStart, saleEnd);
    – Bergi
    Commented Sep 25, 2019 at 7:52
  • 4
    If developers truly want to shoot on their feet, there are easier ways @Bergi. Still, the example you put is far more readable than forTotalAmountAndBreakIntoPaymentsByPeriodThenMonthlyWithPaymentsOfButByDateRange(1234, 2, 12.34, saleStart, saleEnd); Commented Sep 25, 2019 at 17:04
  • 5
    @DanielCuadra The point I was trying to make is that your answer doesn't really solve the OPs problem of having 3 mutually exclusive parameters. Using the builder pattern might make the call more readable (and raise the probability of the user noticing that it doesn't make sense), but using the builder pattern alone doesn't prevent them from still passing 3 values at once.
    – Bergi
    Commented Sep 25, 2019 at 18:23
  • 2
    @Falco Will it? Yes, it's possible, but more complicated, and the answer made no mention of this. The more common builders I've seen consisted of only a single class. If the answer gets edited to include the code of the builder(s), I'll happily endorse it and remove my downvote.
    – Bergi
    Commented Sep 25, 2019 at 21:43
2

Well, in other languages, you'd use named parameters. This can be emulated in Javscript:

function getPaymentBreakdown(total, startDate, durationSpec) { ... }

getPaymentBreakdown(100, today, {endDate: whatever});
getPaymentBreakdown(100, today, {noOfMonths: 4});
getPaymentBreakdown(100, today, {monthlyPayment: 20});
7
  • 6
    Like the builder pattern below, this makes the call more readable (and raises the probability of the user noticing that it doesn't make sense), but naming the parameters does not prevent the user from still passing 3 values at once - e.g. getPaymentBreakdown(100, today, {endDate: whatever, noOfMonths: 4, monthlyPayment: 20}).
    – Bergi
    Commented Sep 25, 2019 at 18:24
  • 1
    Shouldn't it be : instead of =?
    – Barmar
    Commented Sep 25, 2019 at 19:22
  • I guess you could check that only one of the parameters is non-null (or is not in the dictionary). Commented Sep 25, 2019 at 23:43
  • 1
    @Bergi - The syntax itself doesn't prevent users from passing nonsensical parameters but you can simply do some validation and throw errors
    – slebetman
    Commented Sep 26, 2019 at 6:07
  • @Bergi I am by no means a Javascript expert, but I think Destructuring Assignment in ES6 can help here, though I am very light on knowledge with this. Commented Sep 26, 2019 at 6:22
1

As an alternative you could also break the responsibility of specifying the number of month and leave it out of your function :

getPaymentBreakdown(420, numberOfMonths(3))
getPaymentBreakdown(420, dateRage(a, b))
getPaymentBreakdown(420, paymentAmount(350))

And the getpaymentBreakdown would receive an object which would provide the base number of months

Those would higher order function returning for instance a function.

function numberOfMonths(months) {
  return {months: (total) => months};
}

function dateRange(startDate, endDate) {
  return {months: (total) => convertToMonths(endDate - startDate)}
}

function monthlyPayment(amount) {
  return {months: (total) => total / amount}
}


function getPaymentBreakdown(total, {months}) {
  const numMonths= months(total);
  return {
    numMonths, 
    monthlyPayment: total / numMonths,
    endDate: addMonths(startDate, numMonths)
  };
}
3
  • What happened to the total and startDate parameters?
    – Bergi
    Commented Sep 26, 2019 at 17:31
  • This seems like a nice API, but could you please add how you would imagine those four functions to be implemented? (With variant types and a common interface this could be quite elegant, but it's unclear what you had in mind).
    – Bergi
    Commented Sep 26, 2019 at 17:32
  • @Bergi edited my post
    – Vinz243
    Commented Sep 27, 2019 at 8:07
0

And if you were to be working with a system with discriminated unions/algebraic data types, you could pass it in as, say, a TimePeriodSpecification.

type TimePeriodSpecification =
    | DateRange of startDate : DateTime * endDate : DateTime
    | MonthCount of startDate : DateTime * monthCount : int
    | MonthlyPayment of startDate : DateTime * monthlyAmount : float

and then none of the problems would occur where you could fail to actually implement one and so on.

1
  • This is definitely how I would approach this in a language that had types like these available. I tried to keep my question language-agnostic but maybe it should take into account the language used because approaches like this become possible in some cases. Commented Sep 30, 2019 at 16:09

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