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

Refactor TaskState.Pending to be Optional<TaskState>.None #26

Closed
wants to merge 3 commits into from

Conversation

richardjrossiii
Copy link
Member

This allows better guarantees for operations that take a TaskState, such
as being unable to set the state of a task to .Pending anymore.

Depends on #25

This helps for readability, and keepign every file concerned with a
single aspect of 'Task'.
This allows better guarantees for operations that take a TaskState, such
as being unable to set the state of a task to `.Pending` anymore.
@nlutsenko
Copy link
Member

Actually, I don't think I agree with this approach.
The fact that we have Pending as a case for state is an implementation detail, and if you are setting the task to that state internally - you are making a mistake regardless.
You are adding an extra layer of complexity here, where a Pending state is designated with state being set to nil, whilst the finished state is designated with a concrete value.

I can accept it, but let's talk more on why we want this, since it adds complexity for no explicit value.

@richardjrossiii richardjrossiii deleted the richardross.task.state.nil branch June 1, 2016 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants