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

Implement adding a Category in CategoriesUseCase #453

Merged
merged 2 commits into from
Nov 29, 2023

Conversation

PaulKlauser
Copy link
Collaborator

Description:
Implement adding a Category in CategoriesUseCase

@@ -49,10 +46,9 @@ class CategoriesUseCase(
}

override suspend fun addCategory(categoryName: String, sortOrder: Int) {
presetsRepository.addCategory(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the PresetsRepository.addCategory still used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great ask, looks like it's not

@@ -49,10 +46,9 @@ class CategoriesUseCase(
}

override suspend fun addCategory(categoryName: String, sortOrder: Int) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Separate discussions from your code but I'm curious on your thoughts on these sort of mono use cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Meaning, use cases that can do more than one thing? I don't have super strong feelings, and I know a lot of folks go with the one-operation use cases, but I find that doesn't buy you a whole lot for the extra verbosity. What are your thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't buy you much for sure. It feel like its conflating two patterns but doesn't feel too wrong otherwise.

Copy link
Contributor

@IanCrossCD IanCrossCD left a comment

Choose a reason for hiding this comment

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

LGTM

@PaulKlauser PaulKlauser merged commit 7624349 into main Nov 29, 2023
1 check passed
@PaulKlauser PaulKlauser deleted the chore/452-abstract-over-saved-preset-categories branch November 29, 2023 15:12
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