I'm not really happy with the below solution. Background is that I am receiving results from a graphql query that includes tags, and I want to translate these tags into weights and save those results with the weights in my db. Important to note is that in the case where no matching tag is found in a given node, I want to set the default weight.
Currently I put that logic into an enum class that I am then calling from my mapper. I wonder if there is a better way to achieve this. I'm not sure if I like having additional logic in an enum like that. Also I am not happy that my toDto function still contains quite a bit of logic to achieve the final result of assigning a weight to the dto.
enum class Weight(val value: Int) {
LEVEL1(3),
LEVEL2(2),
DEFAULT(1);
companion object {
fun fromTagName(tagName: String): Weight {
return when (tagName) {
"level1" -> LEVEL1
"level2" -> LEVEL2
else -> DEFAULT
}
}
}
}
fun toDto(node: BaseNode): Result<Dto> =
Result.catch {
val weight = node.tags.filter { tag -> tag.name == "tier1" || tag.name == "tier2" }
.map { tag -> Weight.fromTagName(tag.name) }
.firstOrNull()?.value ?: Weight.DEFAULT.value
Dto(
name = provider.name,
weight = weight
)
}
1 Answer 1
Don't overthink this, apart from some minor adjustments I find it acceptable as it is.
There are alternatives, though.
Regarding the fromTagName
function, it is like a factory function for Weight
enums. A natural location for that is the companion object since it is closely related to the enum. If you don't want the code to be part of the class itself you can always extract it as a top-level function like this:
enum class Weight(val value: Int) {
LEVEL1(3),
LEVEL2(2),
DEFAULT(1);
companion object
}
fun Weight.Companion.fromTagName(tagName: String): Weight = when (tagName) {
"level1" -> Weight.LEVEL1
"level2" -> Weight.LEVEL2
else -> Weight.DEFAULT
}
You will still need to keep the companion object
in the class, otherwise you cannot create an extension function for it. Although this function can now be placed everywhere you want I would suggest to keep it near the enum class since both are closely related.
You can use a similar approach for toDto
. You can extract the logic into a top-level function like this:
private fun Iterable<Tag>.weight(): Int {
val firstTag = firstOrNull { tag -> tag.name == "tier1" || tag.name == "tier2" }
return Weight.fromTagName(firstTag?.name).value
}
I also streamlined it a bit: You do not need to map all tags to their weights when you will only use the first one. So I merged the filtering predicate into firstOrNull
to retrieve the first tag that is either "tier1" or "tier2". Then for that tag name the weight value is returned. For the null fallback I adjusted the parameter of fromTagName
to be nullable so it will return DEFAULT
on null:
fun fromTagName(tagName: String?)
This null fallback part of the logic now moved from toDto
to fromTagName
, which seems acceptable to me. It will make the Iterable<Tag>.weight()
function a bit easier to read. Since this new function probably doesn't need to have public visibility I made it private. Adjust as necessary. You can also make it a private member of a class when it is only used there.
Your toDto
function would then look like this:
fun BaseNode.toDto(): Result<Dto> = Result.catch {
Dto(
name = provider.name,
weight = tags.weight(),
)
}
Note that I also changed the BaseNode
parameter to be the receiver of the function. It can then be called on any BaseNode
object like this, which is more fluent:
node.toDto()
I would stop optimizing here.
That said, there is another approach of how to encapsulate logic. Since that seems to be a major motivator of your question I will show you how the above can be done with Use Cases.
A Use Case is basically a single function encapsulated in a class. This has the added benefit of being able to be easily passed around. It also makes the dependencies of a class much more clear since all Use Cases must be provided as a parameter to the constructor. It also creates a single, encapsulated unit of the logic so it can be more easily unit tested.
The above functions would be transformed into Use Cases like this:
class WeightFromTagNameUseCase @Inject constructor() {
operator fun invoke(tagName: String?): Weight = when (tagName) {
"level1" -> Weight.LEVEL1
"level2" -> Weight.LEVEL2
else -> Weight.DEFAULT
}
}
class WeightFromTagsUseCase @Inject constructor(
private val weightFromTagNameUseCase: WeightFromTagNameUseCase,
) {
operator fun invoke(tags: Iterable<Tag>): Int {
val firstTag = tags.firstOrNull { tag -> tag.name == "tier1" || tag.name == "tier2" }
return weightFromTagNameUseCase(firstTag?.name).value
}
}
class BaseNodeToDtoUseCase @Inject constructor(
private val weightFromTagsUseCase: WeightFromTagsUseCase,
) {
operator fun invoke(node: BaseNode): Result<Dto> = Result.catch {
Dto(
name = provider.name,
weight = weightFromTagsUseCase(node.tags),
)
}
}
Note that the functions are now member functions of the surrounding class that only exists for that single function. Moreover, the function is the special invoke
operator. More on that later. Also note that I added the @Inject
annotation to the constructor. When using Use Cases on a larger scale you do not want to manually provide an instance everytime you need it, this would become tedious quite fast. Use the dependency injection framework of your choice to let it handle that for you.
Now let's have a look at the second Use Case, WeightFromTagsUseCase
. The class itself takes a parameter, an instance of the first Use Case WeightFromTagNameUseCase
. That is then used like this:
weightFromTagNameUseCase(firstTag?.name)
It looks like a function call, but weightFromTagNameUseCase
is actually an object. That's the magic of the invoke
operator: It allows an object to be invoked like a function. This is just syntax sugar for calling
weightFromTagNameUseCase.invoke(firstTag?.name)
Since our Use Case object is actually just a wrapper for a function, it is only fair that we now use the object as a function. It makes the code easier to read.
In the same way that we used the first Use Case in the second Use Case, we can also use the second Use Case in the third Use Case.
And just to make it complete: The third Use Case is what you eventually want to use in your code, so let's assume a class where you would call it like this:
class SomeMapper @Inject constructor(
private val baseNodeToDtoUseCase: BaseNodeToDtoUseCase,
) {
fun doStuffWithNode(node: BaseNode) {
val dto: Result<Dto> = baseNodeToDtoUseCase(node)
// do some other stuff with dto
}
}
This may all look a bit overcomplicated, but on larger code bases this can actually simplify the logic because every class that needs a specific part of the logic has to explicitly request it as a parameter. It makes it much more easy to keep track of the dependencies between your code parts. All logic is upgraded from a simple function to an independent object that stands on its own. As you can see above, all its dependencies must also be explicitly stated in its constructor, which helps improve the structure of your logic. Most of the overhead of having an extra class just for a single function can be optimized away by the compiler, so this shouldn't have a significat impact on your performance or memory footprint.
It is, however, by far not as light weight as the functions I initially proposed. I would only recommend using this approach if you want to seriously commit to Use Cases throughout your code base. It's probably only worth it if you already use clean architecture with discrete layers where you can place your Use Cases.
That said, even when adopting Use Cases, in this specific scenario I would leave the first Use Case as a function of the companion object, as it was before. It is too strongly dependent on the enum class itself to separate it from the class.
-
\$\begingroup\$ Thanks a lot for that in depth comment, I learned a lot and will definitely look more into use cases and see if that is something to adopt in my codebase in general! \$\endgroup\$isic5– isic52024年05月13日 08:47:44 +00:00Commented May 13, 2024 at 8:47
-
\$\begingroup\$ One question that I still have is looking at your first suggestion, do you see any clear advantage in splitting the fromTagName and the tags iterable into two separate functions like you did vs. jsut having a fromTags function in my enum as a companion object that directly takes a listOf tags and uses the firstOrNull logic to fetch the matching enum ? \$\endgroup\$isic5– isic52024年05月13日 09:31:21 +00:00Commented May 13, 2024 at 9:31
-
\$\begingroup\$ I would separate that. It is one thing to fetch the proper enum from a String, it is trivial in that it is only a simple technical translation. The other thing is how to iterate a list to extract a value to use. That is a part of the business logic that is non-trivial. The code may be short, but it is different in nature. Someone that doesn't know anything about your program can easily understand what
fromTagName
is and why it is there. That doesn't apply to theweight
function, for that one must have a broader understanding of the context where this function is used. \$\endgroup\$tyg– tyg2024年05月13日 09:51:40 +00:00Commented May 13, 2024 at 9:51