search

alltrust / Reminder-App

Set reminders based importance. Search & filter through passed reminders
0 stars 0 forks source link

"removeReminder" In Place Updates #4

Closed sebastianserrano closed 2 years ago

sebastianserrano commented 2 years ago

Don't mutate existing state, generate a new one. https://medium.com/@kkranthi438/dont-mutate-state-in-react-6b25d5e06f42 https://github.com/alltrust/Reminder-App/blob/628a2d8c14e37277dc36ce95243ad413ef014cd7/src/store/reminders.js#L43

alltrust commented 2 years ago

I'll change the name as it not much remove the reminder from the original array, but changing its completionStatus from false to true.

sebastianserrano commented 2 years ago

I'll change the name as it not much remove the reminder from the original array

my comment wasn't about naming, it was about state management

changing its completionStatus from false to true.

even completionStatus set to a Boolean doesn't make much sense. completed would be more appropriate to a Boolean value. Though, even better would be to have make it an enum. 

enum Statuses {
  completed = 'Completed',
  incomplete = 'Incomplete'
}

type Status = Statuses

OR

type Status = 'Completed' | 'Incomplete'

Good thing about type Status = Statuses is that you can use it later to do Statuses.completed

alltrust commented 2 years ago

9