Skip to content
New issue

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

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

Already on GitHub? # to your account

Update TodoList.vue #2

Open
wants to merge 1 commit into
base: 01-basics
Choose a base branch
from
Open

Update TodoList.vue #2

wants to merge 1 commit into from

Conversation

jakubdomanski
Copy link

126-128 = Support for the Firefox browser without polyfill (Firefox do not support event as global).

Proposition :

    //line 72-80
    if (this.filter == 'all') {
        return this.todos
      } else if (this.filter == 'active') {
        return this.todos.filter(todo => !todo.completed)
      } else if (this.filter == 'completed') {
        return this.todos.filter(todo => todo.completed)
      }
      
      return this.todos

You are returning the same thing with first if and last return the if do not make sense.
This should look like that :

     if (this.filter == 'active') {
        return this.todos.filter(todo => !todo.completed)
      } else if (this.filter == 'completed') {
        return this.todos.filter(todo => todo.completed)
      }
      
      return this.todos

But personally, I'd use switch over if's, just like this :

  switch (this.filter){
    case 'active':
      return this.todos.filter( todo => !todo.completed )
    case 'completed':
      return this.todos.filter( todo => todo.completed )
    }

    return this.todos //still not sure if this should have default return when all we know all cases

Cheers,
Jakub

126-128 = Support for the Firefox browser without polyfill (Firefox do not support event as global).

Proposition : 

```javascript
    //line 72-80
    if (this.filter == 'all') {
        return this.todos
      } else if (this.filter == 'active') {
        return this.todos.filter(todo => !todo.completed)
      } else if (this.filter == 'completed') {
        return this.todos.filter(todo => todo.completed)
      }
      
      return this.todos
```
You are returning the same thing with first if and last return the if do not make sense. 
This should look like that : 
```javascript
     if (this.filter == 'active') {
        return this.todos.filter(todo => !todo.completed)
      } else if (this.filter == 'completed') {
        return this.todos.filter(todo => todo.completed)
      }
      
      return this.todos
```
But personally, I'd use switch over if's, just like this :

```javascript
  switch (this.filter){
    case 'active':
      return this.todos.filter( todo => !todo.completed )
    case 'completed':
      return this.todos.filter( todo => todo.completed )
    }

    return this.todos //still not sure if this should have default return when all we know all cases
```
Cheers, 
Jakub
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant