Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

js課題8 作成しました! #1

Merged
merged 2 commits into from
Jan 11, 2019
Merged

js課題8 作成しました! #1

merged 2 commits into from
Jan 11, 2019

Conversation

rikushingaki
Copy link
Owner

レビューお願いします!

main.js Outdated
@@ -4,8 +4,8 @@
// 1. 掃除
// 2. 買い物
// 3. 散歩
// ここに変数「todos」を用意する

// ここに変数「todo」を用意する

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todos で間違っていいないような気がしますが、todoに変更した理由は何でしょうか?

main.js Outdated
// ここに変数「todos」を用意する

// ここに変数「todo」を用意する
todos = ['1.掃除', '2.買い物', '3.散歩'];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

初めて使う変数の場合はconstかletを使うようにしてください。

main.js Outdated
@@ -15,10 +15,11 @@
// 3. 削除
// 4. 終了
// ここに変数「commands」を用意する
commands = ['確認', '追加', '削除', '終了'];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

main.js Outdated
console.log('========================');
console.log('現在持っているのタスク一覧');
console.log('========================');
if(todos.length < 0) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここの条件がおかしくなっているので、todosの確認をしても常に1件も表示されなあいようになっています。
(現状の実装だとtodos配列の中身が0件未満であればifの中が実行されるようになっています。)

main.js Outdated
@@ -71,6 +96,17 @@
* 3. showTodos関数を実行して、現在保持しているタスク一覧を表示する
*/
// ここにcreateTodo関数を作る
function createTodo() {
todo = prompt('タスクを入力してください');
Copy link

@tsuyopon-xyz tsuyopon-xyz Jan 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

main.js Outdated
@@ -93,4 +129,14 @@
* 3. showTodos関数を実行して、現在保持しているタスク一覧を表示する
*/
// ここにdeleteTodo関数を作る

function deleteTodo() {
NumberString = prompt('削除するタスクの番号を指定してください');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

また変数名は小文字から始めるが一般的です。
(先頭大文字のキャメルケースはクラスに、全て大文字は定数として使われるのが一般的です。)

main.js Outdated

function deleteTodo() {
NumberString = prompt('削除するタスクの番号を指定してください');
parsedNumber = parseInt(NumberString,10);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tsuyopon-xyz
Copy link

いろいろとコメントしたのでご確認お願いしますmm

@tsuyopon-xyz
Copy link

👍

OKです^^
マージしても構いません^^

@rikushingaki rikushingaki merged commit f8800e3 into master Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants