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.

  • 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. – Graham Sep 24 '19 at 20:50
  • 12
    that looks like a Flag Argument anti-pattern, and I would also recommend splitting into several functions – njzk2 Sep 25 '19 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 Sep 25 '19 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). – Ilmari Karonen Sep 25 '19 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 Sep 26 '19 at 13:52
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".

|improve this answer|||||
  • 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? – Giacomo Alzetta Sep 25 '19 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 Sep 25 '19 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. – Giacomo Alzetta Sep 25 '19 at 8:13
  • @GiacomoAlzetta: ok, was a misunderstanding, I thought you were talking about the second implementation of getPaymentBreakdown in the question. – Doc Brown Sep 25 '19 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 Sep 25 '19 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.

|improve this answer|||||
  • 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 - Reinstate Monica Sep 24 '19 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 Sep 25 '19 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 - Reinstate Monica Sep 25 '19 at 14:39
  • @Falco That sounds like a new method to me (on this payment calculator class, not DateInterval): calculatePayPeriod(startData, totalPayment, monthlyPayment) – Alexander - Reinstate Monica Sep 25 '19 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]
|improve this answer|||||
  • 8
    Fluent interface by itself doesn't make any particular task easier or harder. This seems more like the Builder pattern. – VLAZ Sep 25 '19 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 Sep 25 '19 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); – DanielCuadra Sep 25 '19 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 Sep 25 '19 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 Sep 25 '19 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});
|improve this answer|||||
  • 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 Sep 25 '19 at 18:24
  • 1
    Shouldn't it be : instead of =? – Barmar Sep 25 '19 at 19:22
  • I guess you could check that only one of the parameters is non-null (or is not in the dictionary). – Mateen Ulhaq Sep 25 '19 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 Sep 26 '19 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. – Gregory Currie Sep 26 '19 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)
  };
}
|improve this answer|||||
  • What happened to the total and startDate parameters? – Bergi Sep 26 '19 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 Sep 26 '19 at 17:32
  • @Bergi edited my post – Vinz243 Sep 27 '19 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.

|improve this answer|||||
  • 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. – CalMlynarczyk Sep 30 '19 at 16:09

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.