Есть ли шаблон для обработки конфликтующих параметров функции?

38

У нас есть функция API, которая разбивает общую сумму на ежемесячные суммы на основе заданных дат начала и окончания.

// 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,
  };
}

В последнее время потребитель этого API хотел указать диапазон дат другими способами: 1) путем указания количества месяцев вместо даты окончания или 2) путем предоставления ежемесячного платежа и расчета даты окончания. В ответ на это команда API изменила функцию на следующую:

// 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),
  };
}

Я чувствую, что это изменение усложняет API. Теперь абонент должен беспокоиться о эвристиках спрятанных с реализацией функции в определении того, какие параметры принимают предпочтение в настоящее время используется для расчета диапазона дат (т.е. в порядке приоритета monthlyPayment, numMonths, endDate). Если вызывающий не обращает внимания на сигнатуру функции, он может отправить несколько необязательных параметров и запутаться в том, почему endDateигнорируется. Мы определяем это поведение в документации по функциям.

Кроме того, я чувствую, что это создает плохой прецедент и добавляет в API обязанности, которыми он не должен заниматься (то есть нарушает SRP). Пусть дополнительные потребители хотят функции для поддержки большего числа случаев использования, например, вычисления totalот numMonthsи monthlyPaymentпараметров. Эта функция со временем будет становиться все более и более сложной.

Я предпочитаю оставить функцию такой, какой она была, и вместо этого требовать, чтобы вызывающая сторона вычисляла endDateсебя. Тем не менее, я могу ошибаться, и мне было интересно, были ли внесенные изменения приемлемым способом разработки функции API.

В качестве альтернативы, есть ли общий шаблон для обработки подобных сценариев? Мы могли бы предоставить дополнительные функции более высокого порядка в нашем API, которые обертывают исходную функцию, но это раздувает API. Возможно, мы могли бы добавить дополнительный параметр флага, определяющий, какой подход использовать внутри функции.

CalMlynarczyk
источник
79
«В последнее время потребитель этого API хотел [указать] количество месяцев вместо даты окончания» - это несерьезный запрос. Они могут преобразовать число месяцев в правильную дату окончания в строке или двух кода на их конце.
Грэм
12
это похоже на анти-паттерн «Аргумент флага», и я бы также рекомендовал
разбить его
2
Как примечание стороны, есть есть функции , которые могут принимать один и тот же тип и количество параметров и производят очень разные результаты , основанные на тех - см Date- вы можете поставить строку и может быть проанализирована , чтобы определить дату. Однако этот способ обработки параметров также может быть очень привередливым и может привести к ненадежным результатам. Смотри Dateснова. Это не невозможно сделать правильно - Момент справляется с этим лучше, но это очень раздражает в использовании, несмотря на это.
ВЛАЗ
Немного касательно, возможно, вы захотите подумать о том, как обрабатывать случай, в котором monthlyPaymentзадан, но totalне является кратным ему целым числом. А также, как бороться с возможными ошибками округления с плавающей запятой, если значения не гарантируются как целые числа (например, попробуйте с total = 0.3и monthlyPayment = 0.1).
Ильмари Каронен
@ Грэхем, я не отреагировал на это ... Я отреагировал на следующее утверждение: «В ответ на это команда API изменила функцию ...» - сворачивается в положение плода и начинает раскачиваться - не имеет значения, где эта или две строки кода идут либо новым вызовом API с другим форматом, либо выполняются на стороне вызывающего. Только не меняйте рабочий вызов API, как это!
Балдрикк

Ответы:

99

Видя реализацию, мне кажется, что вам действительно нужно 3 разные функции вместо одной:

Оригинальный:

function getPaymentBreakdown(total, startDate, endDate) 

Тот, который предоставляет количество месяцев вместо даты окончания:

function getPaymentBreakdownByNoOfMonths(total, startDate, noOfMonths) 

и тот, который обеспечивает ежемесячный платеж и вычисляет дату окончания:

function getPaymentBreakdownByMonthlyPayment(total, startDate, monthlyPayment) 

Теперь больше нет необязательных параметров, и должно быть достаточно ясно, какая функция называется как и для какой цели. Как упоминалось в комментариях, в строго типизированном языке можно также использовать перегрузку функций, различая 3 разные функции не обязательно по их имени, но по их сигнатуре, в случае, если это не скрывает их назначение.

Обратите внимание, что разные функции не означают, что вы должны дублировать какую-либо логику - внутренне, если эти функции используют общий алгоритм, его следует преобразовать в «приватную» функцию.

Есть ли общий шаблон для обработки сценариев, как это

Я не думаю, что существует «шаблон» (в смысле шаблонов проектирования GoF), который описывает хороший дизайн API. Использование самоописываемых имен, функций с меньшим количеством параметров, функций с ортогональными (= независимыми) параметрами - это только основные принципы создания читаемого, поддерживаемого и развивающегося кода. Не каждая хорошая идея в программировании - это обязательно «шаблон дизайна».

Док Браун
источник
24
На самом деле «обычной» реализацией кода может быть просто getPaymentBreakdown(или на самом деле любая из этих 3), а две другие функции просто преобразуют аргументы и вызывают это. Зачем добавлять частную функцию, которая является идеальной копией одного из этих 3?
Джакомо Альзетта
@GiacomoAlzetta: это возможно. Но я почти уверен, что реализация станет проще, если предоставить общую функцию, которая содержит только часть «return» функции OP, и пусть публичные 3 функции вызывают эту функцию с параметрами innerNumMonths, totalи startDate. Зачем хранить слишком сложную функцию с 5 параметрами, где 3 являются почти необязательными (за исключением того, что нужно установить один), когда функция с 3 параметрами также будет выполнять эту работу?
Док Браун
3
Я не хотел сказать «сохранить функцию 5 аргументов». Я просто говорю, что когда у вас есть общая логика, эта логика не обязательно должна быть частной . В этом случае все 3 функции могут быть реорганизованы для простого преобразования параметров в начальные и конечные даты, поэтому вы можете использовать публичную getPaymentBreakdown(total, startDate, endDate)функцию в качестве общей реализации, а другой инструмент просто вычислит подходящие итоговые / начальные / конечные даты и вызовет ее.
Джакомо Альзетта
@GiacomoAlzetta: хорошо, было недоразумение, я думал, что вы говорите о второй реализации getPaymentBreakdownв вопросе.
Док Браун
Я бы зашел так далеко, что добавил новую версию оригинального метода с явным названием getPaymentBreakdownByStartAndEnd и объявил устаревшим оригинальный метод, если вы хотите предоставить все из них.
Эрик
20

Кроме того, я чувствую, что это создает плохой прецедент и добавляет в API обязанности, которыми он не должен заниматься (то есть нарушает SRP). Пусть дополнительные потребители хотят функции для поддержки большего числа случаев использования, например, вычисления totalот numMonthsи monthlyPaymentпараметров. Эта функция со временем будет становиться все более и более сложной.

Вы совершенно правы.

Я предпочитаю оставить функцию такой, какой она была, и вместо этого требовать, чтобы вызывающая сторона сама рассчитывала endDate. Тем не менее, я могу ошибаться, и мне было интересно, были ли внесенные изменения приемлемым способом разработки функции API.

Это также не идеально, потому что код вызывающего абонента будет загрязнен не связанной котельной плитой.

В качестве альтернативы, есть ли общий шаблон для обработки подобных сценариев?

Введите новый тип, как DateInterval. Добавьте конструкторы, которые имеют смысл (дата начала + дата окончания, дата начала + число месяцев, что угодно.). Примите это как типы единой валюты для выражения интервалов дат / времени в вашей системе.

Александр - Восстановить Монику
источник
3
@DocBrown Да. В таких случаях (Ruby, Python, JS) принято просто использовать статические методы / методы класса. Но это детали реализации, которые я не считаю особенно подходящими для моего ответа («используйте тип»).
Александр - Восстановить Монику
2
И эта идея, к сожалению, достигает своих пределов с третьим требованием: дата начала, общая сумма платежа и ежемесячный платеж - и функция будет вычислять DateInterval из параметров денег - и вы не должны помещать денежные суммы в свой диапазон дат ...
Falco
3
@DocBrown "только переносит проблему из существующей функции в конструктор типа" Да, он помещает временной код туда, куда должен идти временной код, чтобы денежный код мог быть тем, куда должен идти денежный код. Это простой SRP, поэтому я не уверен, к чему вы клоните, когда говорите, что «только» сдвигает проблему. Это то, что делают все функции. Они не убирают код, они перемещают его в более подходящие места. В чем твоя проблема с этим? «но мои поздравления, по крайней мере, 5 апвоттеров поглотили наживку». Это звучит намного более глупо, чем я думаю (надеюсь), что вы намеревались.
Александр - Восстановить Монику
@Falco Для меня это звучит как новый метод (в этом классе калькулятора платежей нет DateInterval):calculatePayPeriod(startData, totalPayment, monthlyPayment)
Александр - Восстановить Монику
7

Иногда беглые выражения помогают в этом:

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();

Если у вас будет достаточно времени для разработки, вы сможете создать надежный API, который работает аналогично доменному языку.

Другим большим преимуществом является то, что IDE с автозаполнением делают почти интегрированные неуместным чтение документации API, а также интуитивно понятны благодаря своим самораскрываемым возможностям.

Есть ресурсы, такие как https://nikas.praninskas.com/javascript/2015/04/26/fluent-javascript/ или https://github.com/nikaspran/fluent.js. по этой теме.

Пример (взят из первой ссылки на ресурс):

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
источник
8
Свободный интерфейс сам по себе не делает какую-либо конкретную задачу легче или сложнее. Это больше похоже на шаблон Builder.
ВЛАЗ
8
Реализация будет довольно сложной, хотя, если вам нужно предотвратить ошибочные вызовы, такие какforTotalAmount(1234).breakIntoPayments().byPeriod(2).monthly().withPaymentsOf(12.34).byDateRange(saleStart, saleEnd);
Берги
4
Если разработчики действительно хотят стрелять на ногах, есть более простые способы @Bergi. Тем не менее, приведенный вами пример гораздо более forTotalAmountAndBreakIntoPaymentsByPeriodThenMonthlyWithPaymentsOfButByDateRange(1234, 2, 12.34, saleStart, saleEnd);
читабелен,
5
@DanielCuadra Суть, которую я пытался подчеркнуть, заключается в том, что ваш ответ на самом деле не решает проблему OP с наличием 3 взаимоисключающих параметров. Использование шаблона компоновщика может сделать вызов более читаемым (и повысить вероятность того, что пользователь заметит, что это не имеет смысла), но использование только шаблона компоновщика не мешает им по-прежнему передавать 3 значения одновременно.
Берги
2
@ Фалько Будет ли это? Да, это возможно, но более сложно, и ответ не упомянул об этом. Более общие строители, которых я видел, состояли только из одного класса. Если ответ будет отредактирован, чтобы включить код строителя (ей), я с радостью одобрю его и удалю свое отрицательное голосование.
Берги
2

Ну, на других языках вы бы использовали именованные параметры . Это можно эмулировать в Javscript:

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

getPaymentBreakdown(100, today, {endDate: whatever});
getPaymentBreakdown(100, today, {noOfMonths: 4});
getPaymentBreakdown(100, today, {monthlyPayment: 20});
Грегори Керри
источник
6
Как и в приведенном ниже шаблоне компоновки, это делает вызов более читабельным (и повышает вероятность того, что пользователь заметит, что это не имеет смысла), но присвоение имен параметрам не мешает пользователю по-прежнему передавать 3 значения одновременно, например getPaymentBreakdown(100, today, {endDate: whatever, noOfMonths: 4, monthlyPayment: 20}).
Берги
1
Разве это не должно быть :вместо =?
Бармар
Я думаю, вы могли бы проверить, что только один из параметров не является нулевым (или отсутствует в словаре).
Матин Улхак
1
@Bergi - Сам синтаксис не мешает пользователям передавать бессмысленные параметры, но вы можете просто выполнить некоторую проверку и
выдать
@ Bergi Я ни в коем случае не эксперт по Javascript, но я думаю, что здесь может помочь Destructuring Assignment в ES6, хотя я очень разбираюсь в этом.
Грегори Керри
1

В качестве альтернативы вы также можете снять ответственность за указание номера месяца и оставить его вне своей функции:

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

И getpaymentBreakdown получит объект, который предоставит базовое количество месяцев

Это будет функция более высокого порядка, возвращающая, например, функцию.

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
источник
Что случилось с totalи startDateпараметров?
Берги
Это похоже на хороший API, но не могли бы вы добавить, как вы представляете эти четыре функции для реализации? (С вариантами вариантов и общим интерфейсом это может быть довольно элегантно, но неясно, что вы имели в виду).
Берги
@Bergi отредактировал мое сообщение
Vinz243
0

И если бы вы работали с системой с различимыми объединениями / алгебраическими типами данных, вы могли бы передать это как, скажем, a TimePeriodSpecification.

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

и тогда ни одна из проблем не возникнет, если вы не сможете реализовать ее и так далее.

NiklasJ
источник
Это определенно то, как я бы подошел к этому на языке, у которого были такие типы, как эти. Я старался, чтобы мой вопрос не зависел от языка, но, возможно, он должен учитывать используемый язык, потому что подобные подходы становятся возможными в некоторых случаях.
CalMlynarczyk