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

Use mapAccumulateFilter #287

Closed
wants to merge 2 commits into from

Conversation

Masynchin
Copy link
Contributor

This is PR in advance, because it relies on adding mapAccumulateFilter to cats (typelevel/cats#4561).

Besides that I have replaced foldLeft in updateOrGiveUp with mapAccumulateFilter, I have also added new clearOrGiveUp method to AppState and use it in Executor.move.

I want to denote two things I have observed:

  1. Is state.updated(task.id, unAssignedTask) same as state.add(updated)? It seems yes for me, but I am not sure.
  2. clearOrGiveUp naming. I have bad intuition in the semantics of the project, so I would be happy if you choose better name for this method.

@Masynchin
Copy link
Contributor Author

I would be happy if you choose better name for this method

Edits by maintainers are allowed

}
candidates.mapAccumulateFilter(state)(_.clearOrGiveUp(_))

def clearOrGiveUp(task: Work.Task): (AppState, Option[Work.Task]) =
Copy link
Member

Choose a reason for hiding this comment

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

this refactor is great, and the name as you mentioned can be improved. I think both updateOrGiveUp and clearOrGiveUp can be renamed to unassignOrGiveup.

@lenguyenthanh
Copy link
Member

The cats library will take a long time to release, I think we can use mapAccumulateFilter when it is available. For now, do you mind to create another PR for clearOrGiveUp/unassignOrGiveup function?

@Masynchin
Copy link
Contributor Author

The cats library will take a long time to release, I think we can use mapAccumulateFilter when it is available. For now, do you mind to create another PR for clearOrGiveUp/unassignOrGiveup function?

Sure, will be in the second

@Masynchin Masynchin mentioned this pull request Mar 13, 2024
@lenguyenthanh
Copy link
Member

@Masynchin would you mind if I close this? You're welcome to create another pr for mapAccumulateFilter when it's ready.

@Masynchin
Copy link
Contributor Author

@Masynchin would you mind if I close this?

Sure, no problemo!

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