PHP Laravel: лучший способ, чем эти операторы if
Мне нужно назначить комиссию на вновь созданную цену. Комиссии могут изменяться в зависимости от клиентов, типов и цен. поэтому, если есть типовая комиссия, она должна сначала получить ее, откатиться к клиентам, откатиться к умолчанию.
мой код работает, но мне кажется, что он слишком "если-у". может есть лучший подход к этому?
private function addDefaultOnlineCommission(Price $price)
{
$defaultCommission = (object)Commission::DEFAULT_COMMISSIONS;
$typeCommission = $price->type->commissions()
->where('is_online', '=', true)->first();
$clientCommission = $price->type->client->commissions()
->where('is_online', '=', true)->first();
if (!$clientCommission && !$typeCommission) {
$commission = $defaultCommission;
}
if ($clientCommission && !$typeCommission) {
$commission = $clientCommission;
}
if ($typeCommission) {
$commission = $typeCommission;
}
$price->commissions()->create([
'commission_type' => $commission->commission_type,
'commission_value' => $commission->commission_value,
'min_value' => $commission->min_value,
'is_online' => true,
'valid_from' => Carbon::now()->format('Y-m-d H:i:s'),
]);
}
благодаря. насчет мнения вы правы, но и в codereview вы получите только одно мнение :)
Ответы 3
Как насчет этого?
if (!$clientCommission && !$typeCommission) {
$commission = $defaultCommission;
} else if ($clientCommission && !$typeCommission) {
$commission = $clientCommission;
} else {
$commission = $typeCommission;
}
Чисто самоуверенный, но я бы справился с этим с помощью нулевых операторов объединения:
$commission = $typeCommission ?? $clientCommission ?? $defaultCommission;
Это будет использовать первое ненулевое значение. Таким образом, он устанавливает приоритет слева направо.
Вы также можете объединить фактические запросы, чтобы не создавать второй, если первый работает.
Я думаю, что функции похожи на «if-y», потому что ваши операторы сложнее, чем они должны быть. Также вы всегда загружаете все резервные копии, что не обязательно в каждом случае. Я бы реорганизовал функцию, чтобы при необходимости загружать только следующий уровень. Это могло выглядеть примерно так:
private function addDefaultOnlineCommission ( Price $price ) {
// load the type commission
$commission = $price->type->commissions()->where( 'is_online', '=', true )->first();
// if there is no type commission, check client commission
if ( empty($commission) ){
$commission = $price->type->client->commissions()->where( 'is_online', '=', true )->first();
}
// if there is no client commission either, fall back to the default
if ( empty($commission) ){
$commission = (object)Commission::DEFAULT_COMMISSIONS;
}
$price->commissions()->create( [
'commission_type' => $commission->commission_type,
'commission_value' => $commission->commission_value,
'min_value' => $commission->min_value,
'is_online' => true,
'valid_from' => Carbon::now()->format( 'Y-m-d H:i:s' ),
] );
}
Это не только сокращает длину вашей функции, но и делает ее более читаемой imho. Кроме того, вы избегаете ненужных запросов.
Примечание. Вы можете заменить empty () на is_null (), поскольку first () возвращает null при пустых результатах.
Спасибо. это похоже на хороший способ избежать запросов.
Я бы сказал, используйте elseifs, но все, что выходит за рамки этого, может быть немного запутанным. Подобный вопрос может иметь тенденцию к формированию мнения, но, возможно, лучше ответить на codereview.