実現したいこと

・共通部分を適切な関数で切り出し命名を行いたい。
・関数部分とメイン部分を分割したい。

発生している問題・分からないこと

共通部分は分かるが、それをどういう関数で定義すれば良いか分からない。

該当のソースコード

php
1<?php
2 
3$english_scores = [98, 38, 89, 84, 54];
4$math_scores = [92, 81, 59, 84, 34, 59];
5$japanese_scores = [57, 68, 19, 74, 94];
6 
7$english_sum = 0;
8foreach($english_scores as $english_score){
9  $english_sum += $english_score;
10}
11$english_average = round($english_sum / count($english_scores), 2);
12 
13$math_sum = 0;
14foreach($math_scores as $math_score){
15  $math_sum += $math_score;
16}
17$math_average = round($math_sum / count($math_scores), 2);
18 
19$japanese_sum = 0;
20foreach($japanese_scores as $japanese_score){
21  $japanese_sum += $japanese_score;
22}
23$japanese_average = round($japanese_sum / count($japanese_scores), 2);
24 
25?>
26<!DOCTYPE html>
27<html lang="ja">
28<head>
29    <title>科目別得点</title>
30</head>
31<body>
32    <h1>科目別得点</h1>
33    <p>英語のスコア合計は<?php echo $english_sum; ?>点、平均<?php echo $english_average; ?>点です。</p>
34    <p>数学のスコア合計は<?php echo $math_sum; ?>点、平均<?php echo $math_average; ?>点です。</p>
35    <p>国語のスコア合計は<?php echo $japanese_sum; ?>点、平均<?php echo $japanese_average; ?>点です。</p>
36</body>
37</html>

試したこと・調べたこと

teratailやGoogle等で検索した
ソースコードを自分なりに変更した
知人に聞いた
その他

上記の詳細・結果

解決策が分からない。

補足

特になし

maisumakun

2024/07/25 08:30

（想定通りに動作しなかったもので構わないので）試しに関数化したコードはないのですか？

行動規範の内容に同意します

回答4件

参考までにデータをjsonにして、HTMLも別ファイルに分けておきます

example.json

json
1{
2"english":[98, 38, 89, 84, 54],
3"math":[92, 81, 59, 84, 34, 59],
4"japanese":[57, 68, 19, 74, 94]
5}

example.htm

html
1<!DOCTYPE html>
2<html lang="ja">
3<head>
4<title>科目別得点</title>
5</head>
6<body>
7<h1>科目別得点</h1>
8<p>英語のスコア合計は%d点、平均%.1f点です。</p>
9<p>数学のスコア合計は%d点、平均%.1f点です。</p>
10<p>国語のスコア合計は%d点、平均%.1f点です。</p>
11</body>
12</html>

example.php

php
1<?php
2$score=json_decode(file_get_contents("example.json"),JSON_OBJECT_AS_ARRAY); 
3$html=file_get_contents("example.htm"); 
4function avg($arr){
5  return array_sum($arr)/count($arr);
6};
7foreach($score as $key=>$val){
8  $sum[$key]=array_sum($val);
9  $avg[$key]=avg($val);
10}
11
12call_user_func_array("printf",
13[$html,
14$sum["english"],$avg["english"],
15$sum["math"],$avg["math"],
16$sum["japanese"],$avg["japanese"],
17]);

投稿2024/07/25 09:22

編集2024/07/25 09:23

yambejp

総合スコア116220

ベストアンサー

今回のコードは、各科目の合計と平均を求める処理が似ていて、繰り返しが多く、少し読みにくくなっています。
もし科目が増えたら、同じようなコードを何度も書かなければなりません。読みやすく、変更しやすく、拡張しやすい という視点で変更してみました。

変更点

・データのまとめ方を変更:
元のコードでは、$english_scores、$math_scores、$japanese_scoresと、科目ごとに別々の変数を使っていました。これらを1つの連想配列$scoresにまとめました。
例えば、科目が増えても'理科' => [98, 38, ...], '社会' => [92, 81, ...]と配列を追加するだけで簡単に対応ができます。

・共通の処理を関数に
元のコードでは、各科目の合計と平均を求める処理が、ほとんど同じように書かれていました。
この共通の処理をcalculateAverageという関数にまとめました。

・HTML部分の繰り返し処理
元のコードでは、各科目の結果を表示する部分が、科目ごとに書かれていました。
foreachという構文を使って、$scoresに含まれるすべての科目に対して、同じ処理を繰り返すようにしました。
これにより、科目が増えても、HTML部分を修正する必要がなくなります。

変更後のコード

php
1<?php
2 
3$scores = [
4    '英語' => [98, 38, 89, 84, 54],
5    '数学' => [92, 81, 59, 84, 34, 59],
6    '国語' => [57, 68, 19, 74, 94],
7　  //科目が増えた場合のイメージです
8    //'理科' => [57, 38, 19, 54, 100],
9    //'社会' => [87, 68, 18, 74, 96],
10];
11 
12function calculateAverage($subject_scores) {
13    $sum = array_sum($subject_scores);
14    $average = round($sum / count($subject_scores), 2);
15    return [$sum, $average];
16}
17 
18?>
19<!DOCTYPE html>
20<html lang="ja">
21<head>
22    <title>科目別得点</title>
23</head>
24<body>
25    <h1>科目別得点</h1>
26    <?php foreach ($scores as $subject => $subject_scores): ?>
27        <?php list($sum, $average) = calculateAverage($subject_scores); ?>
28        <p><?php echo $subject; ?>のスコア合計は<?php echo $sum; ?>点、平均<?php echo $average; ?>点です。</p>
29    <?php endforeach; ?>
30</body>
31</html>

投稿2024/07/25 15:55

elelboo

総合スコア39

aaa233

2024/07/26 08:22

教えて欲しいのですが、 $subject_scores、$sum、$averageは使う前に=0と数値を置かなくて良いんですか。

elelboo

2024/07/26 13:43

変数を使用する前に必ずしも = 0 などで初期化する必要はありません。今回の場合、$subject_scoresは、foreachループ内で$scores配列の値が直接代入されます。 $sumと$averageは、calculateAverage関数の計算結果が代入されます。もし、明確に初期化したいとする場合は、calculateAverage関数で、 function calculateAverage($subject_scores) { $sum = 0; $average = 0; // ... (以降の計算処理は同じ) } とするのがいいかなと思います。

aaa233

2024/07/26 23:48

度々申し訳ないのですが、必ず初期化する必要がある時もありますか。

aaa233

2024/07/29 07:00

回答ありがとうございます。試したところ問題が解決しました！ベストアンサーに選ばせていただきました。

aaa233

2024/07/29 07:13

ベストアンサーに選ばせていただいた後に申し訳ないのですが、foreach構文内の$subjectは科目を指していると思いますが、$subject_scoresと同じように初期化せず、構文内でいきなり使っても出力されますか。

行動規範の内容に同意します

質問文のプログラムは「配列をよく知らない人が書いたプログラム」に見えます。
（他人から配列を与えられてそれを処理するプログラムを書いたことはあるが、自分で「これは配列にすると良い」と考えたことが無い）

・共通部分を適切な関数で切り出し命名を行いたい。

普通は、こういう3回繰り返し処理の場合、
「共通部分を関数にしてそれを別々の3箇所から呼び出す」のでなく、
3回まわるループの中で一度だけその処理を書きます。

array_sum()を使うか、自分でループして足すかは、好き好きで。

悪い例：

PHP
1function foo($subject,$scores){
2    $total = array_sum($scores);
3    echo "$subject は合計 $total 点です。\n";
4}
5
6$english_scores = [98, 38, 89, 84, 54];
7$math_scores = [92, 81, 59, 84, 34, 59];
8$japanese_scores = [57, 68, 19, 74, 94];
9
10foo("英語",$english_scores);
11foo("数学",$math_scores);
12foo("国語",$japanese_scores);

良い例：

PHP
1$all_scores = [
2    "英語" => [98, 38, 89, 84, 54],
3    "数学" => [92, 81, 59, 84, 34, 59],
4    "国語" => [57, 68, 19, 74, 94],
5];
6
7foreach($all_scores as $subject=>$scores){
8    $total = array_sum($scores);
9    echo "$subject は合計 $total 点です。\n";
10}

投稿2024/07/25 12:03

otn

総合スコア85645

配列の合計ならarray_sum()がある。
https://www.php.net/manual/ja/function.array-sum.php
ドキュメントを読んでPHP標準関数を覚えるのが先にやること。
でないと標準関数にある関数を自分で作る無駄なことをしてしまう。

php
1function array_avg(array $scores) {
2    return round(array_sum($scores) / count($scores), 2);
3}
4
5$english_sum = array_sum($english_scores);
6$english_average = array_avg($english_scores);