問題1 適切な変数名

ウェブページの読み込み時間を計測する以下のようなコードがある。変数名についてどのような問題点があるかを指摘した上で書き直せ。

const start = (new Date()).getTime();
// webページを表示する処理(記載省略)
const elapsed = (new Date()).getTime() - start;
console.log(`読み込み時間：${elapsed}秒`);

問題2 イテレータと変数名

次のコードは、学校の生徒のうち学外のクラブに所属している生徒を出力するコードだ。このコードに存在するバグと、そのバグを防ぐためにできる工夫について説明し、コードを書き直せ。

const clubs = [{name: 'soccer', members:[1,3,4]},{name: 'tennis', members:[2,5,6]}, {name: 'swimming', members:[7,8,11]}];
const students = [...Array(10)].map((_, i) => i+1) //1~10

for (let i = 0; i < clubs.length; i++) {
	for (let j = 0; j < clubs[i].members.length; j++) {
		for(let k = 0; k < students.length; k++) {
			if(clubs[i].members[k] == students[j]) {
				console.log(students[j]);
			}
		}
	}
}

問題3 適切なメソッド名とパラメーター

以下のコードは、生徒を表す型と学校のクラスを表すclassを記載したTypeScriptのコードだ。
以下の2つのメソッドを実装せよ。

idが何番から何番までの生徒の配列(型はStudent[])を返す
背の高さに基づいて値を返す関数。ただし、背の高さがA以上B以下までの生徒（Bを含む）と背の高さがA以上B未満までの生徒（Bを含まない）の二通りの返り値(型はStudent[])に対応できる

適宜「//ここに追記せよ」等のコメントを参考としつつ自由に記載してよい。また、二つ目のメソッドは、二通りそれぞれ別のメソッドに分けて実装する(つまり合計3つのメソッドを実装する)方法を取っても良い。

type Student = {
  id: number
  height: number
}

interface SchoolClass {
	//ここに追記せよ
}

class SchoolClassImpl implements SchoolClass {
  constructor(private readonly students: Student[]){}
	//ここに追記せよ
}

const classA = new SchoolClassImpl([{id: 1, height: 150}, {id:3, height: 160}, {id: 5, height: 155}, {id: 6, height: 156}]);
console.log(xxxxxxxxxx); //idが1から5までの生徒の配列を出力
console.log(xxxxxxxxxx); //heightが150以上156以下の生徒の配列を出力
console.log(xxxxxxxxxx); //heightが150以上156未満の生徒の配列を出力

問題4 正確で簡潔なコメント

4-1

ファイルの行数をカウントする関数を書いていたとする。以下のコメントの問題点と改善案の例を書け

// このファイルに含まれる行数を返す
function countLines(fileName: string){...}

4-2

文字列の一部を除去する関数を書いていたとする。以下のコメントの問題点と改善案を書け

// 'src'の先頭や末尾にある'chars'を除去する
function strip(src: string, chars:string){...}

4-3

以下の関数のコメントの問題点と改善案を書け

// 'v'の「要素 < pivot」が「要素 >= pivot」の前に来るように配置し直す
//　それから、「v[i] < pivot」になる最大の'i'を返す（なければ-1を返す）
function partition(arr: number[], pivot:number){...}

問題1-4の解答例

問題1 解答例
getTime()は秒数ではなく、ミリ秒を返すことが分かりにくいという課題があります。そのため、変数名に値の単位であるmsを組み込むことで、分かりやすくなります。また、${elapsed}はミリ秒であるため、ログで出力している文章は不適切となっているので、書き換えるべきです。

const start_ms = (new Date()).getTime();
// webページを表示する処理(記載省略)
const elapsed_ms = (new Date()).getTime() - start;
console.log(`読み込み時間：${elapsed_ms/1000}秒`);

問題2 解答例
if(clubs[i].members[k] == students[j]) の個所は、if(clubs[i].menbers[j] == students[k])が正しい。このバグを防ぐには、より明確な名前をインデックスに付けると良い。i・j・kをclubs_i・members_i・students_iにしたりci・mi・siにする等。

const clubs = [{name: 'soccer', members:[1,3,4]},{name: 'tennis', members:[2,5,6]}, {name: 'swimming', members:[7,8,11]}];
const students = [...Array(10)].map((_, i) => i+1) //1~10

for (let ci = 0; ci < clubs.length; ci++) {
	for (let mi = 0; mi < clubs[ci].members.length; mi++) {
		for(let si = 0; si < students.length; si++) {
			if(clubs[ci].members[mi] == students[si]) {
				console.log(students[si]);
			}
		}
	}
}

また、リーダブルコードの書籍では触れられていませんが、そもそも for 文を使用すると、深い入れ子構造になるという課題もあります。そのため、JavaScript の配列が持つメソッドを使用して解決する別の解決策もあり得ます。

const clubs = [{name: 'soccer', members:[1,3,4]},{name: 'tennis', members:[2,5,6]}, {name: 'swimming', members:[7,8,11]}];
const students = [...Array(10)].map((_, i) => i+1) //1~10

const everybodyInClub = clubs.flatMap((club) => club.members);
const studentsInClub = everybodyInClub.filter((member)=> students.includes(member));
console.log(studentsInClub.join('\n'));

問題3 解答例
様々な実装方法がありますが、一例としては下記になります。

type Student = {
  id: number
  height: number
}

interface SchoolClass {
  selectBetweenId:(params: {first:number, last:number})=> Student[]
  selectBetweenHeight: SelectBetweenHeightFn
}

interface SelectBetweenHeightFn {
  (params: {min: number, max: number}): Student[]
  (params: {begin: number, end: number}): Student[]
}

class SchoolClassImpl implements SchoolClass {
  constructor(private readonly students: Student[]){}
  selectBetweenId = ({first, last}) => {
      return this.students.filter(student => first <= student.id && student.id <= last);
  }
  selectBetweenHeight = (params) => {
      if(validateMinMaxParams(params)){
          return this.students.filter(student => params.min <= student.height && student.height <= params.max);
      }
      return this.students.filter(student => params.begin <= student.height && student.height < params.end);
  }
}

const validateMinMaxParams = (params)=> {
  const paramKeys = Object.keys(params);
  const targetKeys = ['min', 'max'];
  return targetKeys.every(targetKey => paramKeys.includes(targetKey));
}

const classA = new SchoolClassImpl([{id: 1, height: 150}, {id:3, height: 160}, {id: 5, height: 155}, {id: 6, height: 156}]);
console.log(classA.selectBetweenId({first: 1, last: 5}));
console.log(classA.selectBetweenHeight({min: 150, max:156}));
console.log(classA.selectBetweenHeight({begin: 150, end:156}));

順番に解説していきます。

interface SchoolClass {
  selectBetweenId:(params: {first:number, last:number})=> Student[]
  selectBetweenHeight: SelectBetweenHeightFn
}

この個所については、まずメソッド名についてはselectという単語を使うことで、パラメーターで指定した条件のみを選択するのか、パラメーターで指定した条件のものを排除するのかどちらかが明確になるようにしました。(もし排除する場合はexcludeが適切。filterでは選択か排除かが分からない)

また、selectBetweenIdについては範囲を指定しているので、firstとlastというkeyを持つオブジェクトをパラメータに持つようにしました。パラメーターをオブジェクトにせずに、以下のようにするとメソッドの内容が分かりづらくなる点に注意。

selectBetweenId:(number, number)=> Student[]

別解としては、first, second以外でもmin, maxというkeyのオブジェクトをパラメータに指定するようにしても良かったかと思います。studentsは基本的にidが昇順の配列となるという想定のもと、今回はfirstとsecondを用いました。順番が滅茶苦茶の場合は、minとmaxという限界値を表す表現を用いた方が良いでしょう。

interface SelectBetweenHeightFn {
  (params: {min: number, max: number}): Student[]
  (params: {begin: number, end: number}): Student[]
}

ここではA以上B以下の場合は、minとmaxという限界値を表す表現を用いるべきです。一方で、A以上B未満という含有/排他的範囲を表すにはbeginとendを用いるのが一般的です。

問題4-1 解答例

行の意味には様々なものがあるという課題がある。

""(空のファイル)は0行なのか1行なのか
"hello"は0行なのか1行なのか
"hello\n"は1行なのか2行なのか
"hello\n world"は1行なのか2行なのか
"hello\n\r cruel\n world\r"は、2行なのか3行なのか4行なのか

一番シンプルなのは、\nをカウントするもので、以下のようにコメントすればいい

// このファイルに含まれる改行文字('\n')を数える
function countLines(fileName: string){...}

問題4-2 解答例
charsの意味合いが分かりづらいという課題がある

charsは、除去する文字列なのか、順序のない文字集合なのか？
srcの末尾に複数のcharsがあったらどうなるのか？

例えば、以下のようなコメント方法がある

// 実例：strip('abba/a/ba', 'ab')は"/a/"を返す
function strip(src: string, chars:string){...}

問題4-3 解答例
コメントだけでは視覚化が難しいので、以下の様に実例のコメントを足すという方法がある

// 'v'の「要素 < pivot」が「要素 >= pivot」の前に来るように配置し直す
//　それから、「v[i] < pivot」になる最大の'i'を返す（なければ-1を返す）
// 実例：Partition([8,5,9,8,2], 8)の結果は[5, 2, 8, 9, 8]となり、1を返す
function partition(arr: number[], pivot:number){...}

＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝

解説は以上となります！

リーダブルコードの残り部分の問題集である第2弾はこちらです！

いいねやストックよろしくお願いします！