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.

CalMlynarczyk
  • 491
  • 4
  • 7
  • 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 06: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`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/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 06: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 08: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
  • This is not possible in JavaScript, except by passing a dictionary. [But it is possible in Python using named keyword arguments.](https://treyhunner.com/2018/04/keyword-arguments-in-python/) – noɥʇʎԀʎzɐɹƆ Sep 26 '19 at 21:27

6 Answers6

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".

Doc Brown
  • 199,015
  • 33
  • 367
  • 565
  • 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 07: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 07: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 08: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 09: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
  • I really like this answer. Just one quible: I just got done arguing against [the brain dead form of DRY](https://softwareengineering.stackexchange.com/a/398745/131624). "Logic" sounds like implementation. How about a minor rewrite? "...share some common responsibility, it could be refactored to ..." – candied_orange Sep 25 '19 at 14:23
  • @candied_orange: I know precisely what you mean, but speaking about "refactoring reponsibilities" sounds very unsual to me - I doubt this would improve my answer. In the end, refactoring is applied to code, to some implementation, not to "some responsibility" (but justified by the latter). And in this example case, I think it is pretty clear the common function will correspond to a common responsibility. – Doc Brown Sep 25 '19 at 15:20
  • Common duties? Common ideas? I don't know. Just please save me from the visual diff demons. – candied_orange Sep 25 '19 at 15:34
  • Note that if your consumers require all three functions to share the same name, this can be simplified by providing a frontend that takes four arguments and delegates to the correct worker, where the third argument is a generic standin for all the "end" parameters, and the fourth argument is a flag indicating which version to call (and defaulting to the `endDate` version). Also note that if your consumers require all three functions to share the same name, I feel so very sorry for you. – Justin Time - Reinstate Monica Sep 25 '19 at 19:38
  • While the OP programs in JS, I feel that this question needs at least a mention of function overloading. And everything else which should be mentioned in such an answer is already here, so @DocBrown could I persuade you to add such a mention? – jaskij Sep 25 '19 at 20:51
  • @JanDorniak: sure, when using a strictly typed language, one could in theory try to utilize overloading. But would that really help to make this clearer for this specific case? For other cases, it could be a good fit, of course. – Doc Brown Sep 25 '19 at 21:15
  • @DocBrown for the start end date and number of months cases it would, at least from my perspective. The monthly payment case is entirely different and should not be overloaded. Probably couldn't even be overloaded to be different enough from the number of month case, at least in C++. – jaskij Sep 25 '19 at 21:18
  • @JanDorniak: ok, I mentioned function overloading. Satisfied? – Doc Brown Sep 25 '19 at 21:26
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.

Alexander
  • 3,562
  • 1
  • 19
  • 24
  • 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 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 07: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 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 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]
DanielCuadra
  • 277
  • 1
  • 4
  • 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 06:25
  • It allows adding more fluent methods that can shift the path to different outcomes, instead of accepting a generic parameter and start figuring out whether it's a date or string or array, etc. Builder pattern's sidekick is fluent-expressions, which is why both are typically seemed as one. – DanielCuadra Sep 25 '19 at 06:38
  • +1 For 3 or 4 signatures this would not make a huge difference, but when you have 6+ signatures for different combinations the fluent/builder pattern will make it easier to use in my opinion than just 6+ different really long method names like `getPaymentBreakdownByMonthCalculatingTotalwithCustomInterestFromPeriod` – Falco Sep 25 '19 at 07:48
  • 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 07: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
  • @Bergi yes it will, since the builder will return different classes, which only provide methods for the right arguments – Falco Sep 25 '19 at 21:36
  • 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
    @Bergi, the examples provided by OP are in Javascript; that language doesn't enforce parameters, so devs can wrongly pass 0, 3 or 100 parameters 24/7. There's no pattern that can prevent JS from doing that. The OP asked `Is there a pattern for handling conflicting function parameters?` and yes, builder+fluent-interface patterns elegantly slice the choices such that these params-conflict-issues are prevented. See the updated response. – DanielCuadra Sep 25 '19 at 22:13
  • 1
    An example to experience this: Paste the following code into a Chrome Console and you will get intellisense for an API which is made up of some easy lambdas: `let paymentBreakdown = { forAmount: amountInUsd => ({ byDays: days => days*amountInUsd, byRange: (startDate,endDate) => (endDate.getTime()-startDate.getTime())/(24*3600*1000) * amountInUsd, withPaymentsOf: partialAmount => ({ daily: () => partialAmount*amountInUsd, monthly : () => partialAmount*30*amountInUsd})}) }` Just start typing `paymentBreakdown` and autocomplete through the API – Falco Sep 27 '19 at 12:03
  • This is a really interesting approach that I never considered. I don't know if it's something that my team can utilize currently, but it's definitely something I will keep in mind for future implementations. – CalMlynarczyk Sep 30 '19 at 16:07
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});
slebetman
  • 1,394
  • 9
  • 9
  • 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 06: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 06:22
  • @slebetman Well then *that* should be the answer - and of course you can also do the validation and error throwing if the parameters are not named. – Bergi Sep 26 '19 at 08:56
  • @GregoryCurrie Destructuring only simplifies the declaration of the "named parameters", it helps with nothing else. You still just got three variables that might or might not have a value inside the function body. – Bergi Sep 26 '19 at 08:58
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)
  };
}
Vinz243
  • 217
  • 1
  • 9
  • 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 08: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.

NiklasJ
  • 675
  • 4
  • 6
  • 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