Closed5ヶ月前にクローズ1

「ムーザルちゃんねる:プログラミングで良い関数を作るために大切なポイント」を観た

PHP

YouTube

HTMLGO

元動画

観ていてとてもいいこと言ってるなー、と思ったのでメモ。
それぞれの再生時間のリンクを貼ってます。
↓↓↓↓↓↓↓↓↓↓↓

「名前が抽象的すぎる」

基本、関数の中を読まなくても、名前だけで理解できる方がいいよね。

「名前が複雑でわかりにくい」

１つの関数に色々な役割を詰め込み過ぎるやつ。よくやってしまう。
１つの関数は１つの役割にする、などした方が再利用しやすいし、テストも書きやすい。

「名前から想像する挙動と違う」

getData → 外部リソースに依存せず、即時処理
fetchData → 外部リソースに依存する、時間がかる処理

これは目から鱗。
get使いがちだったので、fetchとちゃんと区別して書こうと思う。

仕様変更に伴って、関数名をちゃんと変えようという話。

create → upsert (insertとupdateを両用やる、というのでよく使うらしい)

「引数が6個以上ある」

こういうメソッドは個人的にも好きじゃ無い。
引数の順番が把握しずらい。

自分の場合は、max３個くらいまでかなー。
オブジェクトにしてしまう派。

function update($a,$b,$c,$d,$e,$f){}

function update($object){
 // $object->a
}

「複数の役割が混じっている」

無理に１つにまとめない。
抽象化したつもりが、逆に複雑になって読みづらくなる。
多少処理が被っていても、別関数で分けちゃった方がいい場合もある。
動画でも言っているが、この辺は確かにセンスが要るかもしれない。

「returnが必ず最後」

早期リターンは完全同意！

演習問題

自分でやってみた。
メソッド無しでやったやつ


<?php

$cart = [
 ['item' => 'お酒', 'price' => 300, 'quantity' => 4],
 ['item' => 'りんご', 'price' => 100, 'quantity' => 2],
];

$taxList = [
    'お酒' => 10,
    'りんご' => 8
];

$total = 0;

foreach($cart as $child){
    $rate = $taxList[$child['item']] / 100;
    $total += $child['price'] * $child['quantity'] * (1 + $rate);
}

echo $total;
// 1536

このままでもいい気がするが、練習のために関数化してみる。

<?php

$cart = [
 ['item' => 'お酒', 'price' => 300, 'quantity' => 4],
 ['item' => 'りんご', 'price' => 100, 'quantity' => 2],
];

function getTaxList(){
    return [
        'お酒' => 10,
        'りんご' => 8
    ];
}

// 商品の税率を取得
function getTaxRate($taxList, $name, $isEatIn){
    if($isEatIn) return 0.1;
    return $taxList[$name] / 100;
}

// 商品ごとの小計を取得
function getItemTotalPrice($item, $taxRate){
    return $item['price'] * $item['quantity'] * (1 + $taxRate);
}

// 全てのtotalの計算
function getTotalPrice($cart, $isEatIn=false){
    $total = 0;
    foreach($cart as $item){
        $taxRate = getTaxRate(getTaxList(), $item['item'], $isEatIn);
        $total += getItemTotalPrice($item, $taxRate);
    }
    return $total;
}

// 1536
echo getTotalPrice($cart);
echo '<br>';
echo getTotalPrice($cart,true);

商品ごとの小計を取得 → getItemTotalPriceという名前が微妙だ。
動画だと calcSubTotal　としていた。なるほどー。

このtaxListの配列を引数として渡したくない、というのもなんかわかる。
というわけで修正した。
↓↓↓↓↓↓↓↓↓↓↓v↓


function getTaxList(){
    return [
        'お酒' => 10,
        'りんご' => 8
    ];
}

// 商品の税率を取得
function getTaxRate($name, $isEatIn){
    if($isEatIn) return 0.1;
    return getTaxList()[$name] / 100;
}

// 全てのtotalの計算
function getTotalPrice($cart, $isEatIn=false){
    $total = 0;
    foreach($cart as $item){
        $taxRate = getTaxRate($item['item'], $isEatIn);
        $total += getItemTotalPrice($item, $taxRate);
    }
    return $total;
}

このスクラップは5ヶ月前にクローズされました