Skip to main content
Code Review

Return to Answer

replaced https://vuejs.org/v2 with https://v2.vuejs.org/v2
Source Link

Is the application well-structured?

On the whole it seems okay, though see the answer to the next question that means the structure could be slightly changed for the better.

Could the code be significantly "shortened"?

Simon Says: use computed properties

Like Simon suggests: use computed properties - the implementation inside getUnsolvedTodos() could be moved to a computed property, with a return instead of assigning the result from calling .filter() to a data variable. Then there is no need to need to call that method and set up the property within the object returned by the data function.

Promise callback consolidation

The call to axios.get() in showTodos() has multiple .then() callbacks:

showTodos: function(){ 
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 })
 .then(this.getUnsolvedTodos)
 .then(this.dataIsLoaded = true);
 },

Those can be consolidated to a single callback - especially since none of them return a promise.

showTodos: function(){
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 this.getUnsolvedTodos(); //this can be removed - see previous section
 this.dataIsLoaded = true;
 });
},

While this does require one extra line, it would avoid confusion because somebody reading the code might think the statements passed to .then() should be functions that return promises.

Single-use variables

In toggleTodo the variable newStatus is only used once so it could be consolidated into the object passed to the call:

axios.put(`${this.apiUrl}/todo/update/${todo.id}`, {
 title: todo.title,
 completed: todo.completed == "0" ? 1 : 0
})

If that variable is kept it could be created with const instead of let since it is never re-assigned.

Passing events to parent

In List.vue the <TodoItem has these attributes:

@delete-todo="$emit('delete-todo', todo.id)"
@toggle-todo="$emit('toggle-todo', todo)"

Those seem redundant. In Vue 2 these lines could be replaced with a single line: v-on="$listeners" v-on="$listeners" but apparently that was removed with Vue3. I tried replacing those lines with v-bind="$attrs" but it didn't seem to work - I will search for the VueJS 3 way to do this.

Is the application well-structured?

On the whole it seems okay, though see the answer to the next question that means the structure could be slightly changed for the better.

Could the code be significantly "shortened"?

Simon Says: use computed properties

Like Simon suggests: use computed properties - the implementation inside getUnsolvedTodos() could be moved to a computed property, with a return instead of assigning the result from calling .filter() to a data variable. Then there is no need to need to call that method and set up the property within the object returned by the data function.

Promise callback consolidation

The call to axios.get() in showTodos() has multiple .then() callbacks:

showTodos: function(){ 
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 })
 .then(this.getUnsolvedTodos)
 .then(this.dataIsLoaded = true);
 },

Those can be consolidated to a single callback - especially since none of them return a promise.

showTodos: function(){
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 this.getUnsolvedTodos(); //this can be removed - see previous section
 this.dataIsLoaded = true;
 });
},

While this does require one extra line, it would avoid confusion because somebody reading the code might think the statements passed to .then() should be functions that return promises.

Single-use variables

In toggleTodo the variable newStatus is only used once so it could be consolidated into the object passed to the call:

axios.put(`${this.apiUrl}/todo/update/${todo.id}`, {
 title: todo.title,
 completed: todo.completed == "0" ? 1 : 0
})

If that variable is kept it could be created with const instead of let since it is never re-assigned.

Passing events to parent

In List.vue the <TodoItem has these attributes:

@delete-todo="$emit('delete-todo', todo.id)"
@toggle-todo="$emit('toggle-todo', todo)"

Those seem redundant. In Vue 2 these lines could be replaced with a single line: v-on="$listeners" but apparently that was removed with Vue3. I tried replacing those lines with v-bind="$attrs" but it didn't seem to work - I will search for the VueJS 3 way to do this.

Is the application well-structured?

On the whole it seems okay, though see the answer to the next question that means the structure could be slightly changed for the better.

Could the code be significantly "shortened"?

Simon Says: use computed properties

Like Simon suggests: use computed properties - the implementation inside getUnsolvedTodos() could be moved to a computed property, with a return instead of assigning the result from calling .filter() to a data variable. Then there is no need to need to call that method and set up the property within the object returned by the data function.

Promise callback consolidation

The call to axios.get() in showTodos() has multiple .then() callbacks:

showTodos: function(){ 
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 })
 .then(this.getUnsolvedTodos)
 .then(this.dataIsLoaded = true);
 },

Those can be consolidated to a single callback - especially since none of them return a promise.

showTodos: function(){
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 this.getUnsolvedTodos(); //this can be removed - see previous section
 this.dataIsLoaded = true;
 });
},

While this does require one extra line, it would avoid confusion because somebody reading the code might think the statements passed to .then() should be functions that return promises.

Single-use variables

In toggleTodo the variable newStatus is only used once so it could be consolidated into the object passed to the call:

axios.put(`${this.apiUrl}/todo/update/${todo.id}`, {
 title: todo.title,
 completed: todo.completed == "0" ? 1 : 0
})

If that variable is kept it could be created with const instead of let since it is never re-assigned.

Passing events to parent

In List.vue the <TodoItem has these attributes:

@delete-todo="$emit('delete-todo', todo.id)"
@toggle-todo="$emit('toggle-todo', todo)"

Those seem redundant. In Vue 2 these lines could be replaced with a single line: v-on="$listeners" but apparently that was removed with Vue3. I tried replacing those lines with v-bind="$attrs" but it didn't seem to work - I will search for the VueJS 3 way to do this.

added 1 character in body
Source Link

Is the application well-structured?

On the whole it seems okay, though see the answer to the next question that means the structure could be slightly changed for the better.

Could the code be significantly "shortened"?

Simon Says: use computed properties

Like Simon suggests: use computed properties - the implementation inside getUnsolvedTodos() could be moved to a computed property, with a return instead of assigning the result from calling .filter() to a data variable. Then there is no need to need to call that method and set up the property within the object returned by the data function.

Promise callback consolidation

The call to axios.get() in showTodos() has multiple .then() callbacks:

showTodos: function(){> 
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 })
 .then(this.getUnsolvedTodos)
 .then(this.dataIsLoaded = true);
 },

Those can be consolidated to a single callback - especially since none of them return a promise.

showTodos: function(){
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 this.getUnsolvedTodos(); //this can be removed - see previous section
 this.dataIsLoaded = true;
 });
},

While this does require one extra line, it would avoid confusion because somebody reading the code might thingthink the statements passed to .then() should be functions that return promises.

Single-use variables

In toggleTodo the variable newStatus is only used once so it could be consolidated into the object passed to the call:

axios.put(`${this.apiUrl}/todo/update/${todo.id}`, {
 title: todo.title,
 completed: todo.completed == "0" ? 1 : 0
})

If that variable is kept it could be created with const instead of let since it is never re-assigned.

Passing events to parent

In List.vue the <TodoItem has these attributes:

@delete-todo="$emit('delete-todo', todo.id)"
@toggle-todo="$emit('toggle-todo', todo)"

Those seem redundant. In Vue 2 these lines could be replaced with a single line: v-on="$listeners" but apparently that was removed with Vue3. I tried replacing those lines with `v-bind="$attrs"v-bind="$attrs" but it didn't seem to work - I will search for the VueJS 3 way to do this.

Is the application well-structured?

On the whole it seems okay, though see the answer to the next question that means the structure could be slightly changed for the better.

Could the code be significantly "shortened"?

Simon Says: use computed properties

Like Simon suggests: use computed properties - the implementation inside getUnsolvedTodos() could be moved to a computed property, with a return instead of assigning the result from calling .filter() to a data variable. Then there is no need to need to call that method and set up the property within the object returned by the data function.

Promise callback consolidation

The call to axios.get() in showTodos() has multiple .then() callbacks:

showTodos: function(){> 
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 })
 .then(this.getUnsolvedTodos)
 .then(this.dataIsLoaded = true);
 },

Those can be consolidated to a single callback - especially since none of them return a promise.

showTodos: function(){
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 this.getUnsolvedTodos(); //this can be removed - see previous section
 this.dataIsLoaded = true;
 });
},

While this does require one extra line, it would avoid confusion because somebody reading the code might thing the statements passed to .then() should be functions that return promises.

Single-use variables

In toggleTodo the variable newStatus is only used once so it could be consolidated into the object passed to the call:

axios.put(`${this.apiUrl}/todo/update/${todo.id}`, {
 title: todo.title,
 completed: todo.completed == "0" ? 1 : 0
})

If that variable is kept it could be created with const instead of let since it is never re-assigned.

Passing events to parent

In List.vue the <TodoItem has these attributes:

@delete-todo="$emit('delete-todo', todo.id)"
@toggle-todo="$emit('toggle-todo', todo)"

Those seem redundant. In Vue 2 these lines could be replaced with a single line: v-on="$listeners" but apparently that was removed with Vue3. I tried replacing those lines with `v-bind="$attrs" but it didn't seem to work - I will search for the VueJS 3 way to do this.

Is the application well-structured?

On the whole it seems okay, though see the answer to the next question that means the structure could be slightly changed for the better.

Could the code be significantly "shortened"?

Simon Says: use computed properties

Like Simon suggests: use computed properties - the implementation inside getUnsolvedTodos() could be moved to a computed property, with a return instead of assigning the result from calling .filter() to a data variable. Then there is no need to need to call that method and set up the property within the object returned by the data function.

Promise callback consolidation

The call to axios.get() in showTodos() has multiple .then() callbacks:

showTodos: function(){ 
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 })
 .then(this.getUnsolvedTodos)
 .then(this.dataIsLoaded = true);
 },

Those can be consolidated to a single callback - especially since none of them return a promise.

showTodos: function(){
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 this.getUnsolvedTodos(); //this can be removed - see previous section
 this.dataIsLoaded = true;
 });
},

While this does require one extra line, it would avoid confusion because somebody reading the code might think the statements passed to .then() should be functions that return promises.

Single-use variables

In toggleTodo the variable newStatus is only used once so it could be consolidated into the object passed to the call:

axios.put(`${this.apiUrl}/todo/update/${todo.id}`, {
 title: todo.title,
 completed: todo.completed == "0" ? 1 : 0
})

If that variable is kept it could be created with const instead of let since it is never re-assigned.

Passing events to parent

In List.vue the <TodoItem has these attributes:

@delete-todo="$emit('delete-todo', todo.id)"
@toggle-todo="$emit('toggle-todo', todo)"

Those seem redundant. In Vue 2 these lines could be replaced with a single line: v-on="$listeners" but apparently that was removed with Vue3. I tried replacing those lines with v-bind="$attrs" but it didn't seem to work - I will search for the VueJS 3 way to do this.

added 559 characters in body
Source Link

Is the application well-structured?

On the whole it seems okay, though see the answer to the next question that means the structure could be slightly changed for the better.

Could the code be significantly "shortened"?

Simon Says: use computed properties

Like Simon suggests: use computed properties - the implementation inside getUnsolvedTodos() could be moved to a computed property, with a return instead of assigning the result from calling .filter() to a data variable. Then there is no need to need to call that method and set up the property within the object returned by the data function.

Promise callback consolidation

The call to axios.get() in showTodos() has multiple .then() callbacks:

showTodos: function(){> 
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 })
 .then(this.getUnsolvedTodos)
 .then(this.dataIsLoaded = true);
 },

Those can be consolidated to a single callback - especially since the first two don'tnone of them return a promise.

showTodos: function(){
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 this.getUnsolvedTodos(); //this can be removed - see previous section
 this.dataIsLoaded = true;
 });
},

While this does require one extra line, it would avoid confusion because somebody reading the code might thing the statements passed to .then() should be functions that return promises.

Single-use variables

In toggleTodo the variable newStatus is only used once so it could be consolidated into the object passed to the call:

axios.put(`${this.apiUrl}/todo/update/${todo.id}`, {
 title: todo.title,
 completed: todo.completed == "0" ? 1 : 0
})

If that variable is kept it could be created with const instead of let since it is never re-assigned.

Passing events to parent

In List.vue the <TodoItem has these attributes:

@delete-todo="$emit('delete-todo', todo.id)"
@toggle-todo="$emit('toggle-todo', todo)"

Those seem redundant. In Vue 2 these lines could be replaced with a single line: v-on="$listeners" but apparently that was removed with Vue3 . I tried replacing those lines with `v-bind="$attrs" but it didn't seem to work - I will search for the VueJS 3 way to do this.

Is the application well-structured?

On the whole it seems okay, though see the answer to the next question that means the structure could be slightly changed for the better.

Could the code be significantly "shortened"?

Simon Says: use computed properties

Like Simon suggests: use computed properties - the implementation inside getUnsolvedTodos() could be moved to a computed property, with a return instead of assigning the result from calling .filter() to a data variable. Then there is no need to need to call that method and set up the property within the object returned by the data function.

Promise callback consolidation

The call to axios.get() in showTodos() has multiple .then() callbacks:

showTodos: function(){> 
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 })
 .then(this.getUnsolvedTodos)
 .then(this.dataIsLoaded = true);
 },

Those can be consolidated to a single callback - especially since the first two don't return a promise.

showTodos: function(){
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 this.getUnsolvedTodos(); //this can be removed - see previous section
 this.dataIsLoaded = true;
 });
},

While this does require one extra line, it would avoid confusion because somebody reading the code might thing the statements passed to .then() should be functions that return promises.

Single-use variables

In toggleTodo the variable newStatus is only used once so it could be consolidated into the object passed to the call:

axios.put(`${this.apiUrl}/todo/update/${todo.id}`, {
 title: todo.title,
 completed: todo.completed == "0" ? 1 : 0
})

If that variable is kept it could be created with const instead of let since it is never re-assigned.

Is the application well-structured?

On the whole it seems okay, though see the answer to the next question that means the structure could be slightly changed for the better.

Could the code be significantly "shortened"?

Simon Says: use computed properties

Like Simon suggests: use computed properties - the implementation inside getUnsolvedTodos() could be moved to a computed property, with a return instead of assigning the result from calling .filter() to a data variable. Then there is no need to need to call that method and set up the property within the object returned by the data function.

Promise callback consolidation

The call to axios.get() in showTodos() has multiple .then() callbacks:

showTodos: function(){> 
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 })
 .then(this.getUnsolvedTodos)
 .then(this.dataIsLoaded = true);
 },

Those can be consolidated to a single callback - especially since none of them return a promise.

showTodos: function(){
 axios.get(`${this.apiUrl}/todos`)
 .then((response) => {
 this.todos = response.data;
 this.getUnsolvedTodos(); //this can be removed - see previous section
 this.dataIsLoaded = true;
 });
},

While this does require one extra line, it would avoid confusion because somebody reading the code might thing the statements passed to .then() should be functions that return promises.

Single-use variables

In toggleTodo the variable newStatus is only used once so it could be consolidated into the object passed to the call:

axios.put(`${this.apiUrl}/todo/update/${todo.id}`, {
 title: todo.title,
 completed: todo.completed == "0" ? 1 : 0
})

If that variable is kept it could be created with const instead of let since it is never re-assigned.

Passing events to parent

In List.vue the <TodoItem has these attributes:

@delete-todo="$emit('delete-todo', todo.id)"
@toggle-todo="$emit('toggle-todo', todo)"

Those seem redundant. In Vue 2 these lines could be replaced with a single line: v-on="$listeners" but apparently that was removed with Vue3 . I tried replacing those lines with `v-bind="$attrs" but it didn't seem to work - I will search for the VueJS 3 way to do this.

added 942 characters in body
Source Link
Loading
Source Link
Loading
lang-js

AltStyle によって変換されたページ (->オリジナル) /