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

fix: Implementation for Equals & Hashcode in Builder #968

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Distractic
Copy link
Contributor

@Distractic Distractic commented Aug 15, 2024

Context

When we compare the builder, like EmbedBuilder, the comparison between two instances will always return false no matter the field set. For example:

println(EmbedBuilder() == EmbedBuilder()) // false
println(EmbedBuilder().apply { title = "title" } == EmbedBuilder().apply { title = "title" }) // false

println(EmbedBuilder().hashCode()) // 2096057945
println(EmbedBuilder().hashCode()) // 1689843956

This behavior seems to be a bug and needs to be fixed because according to the result, equals & hashcode methods don't work.

Usage

In my bot, when I render my components, I would like to compare the last InteractionResponseModifyBuilder and the new from the rendering to send only the changed components (embed / actionRow / text) to the discord API and consequently reduce the network load of the bot

After change

If we try the previous code in this PR, this is the result:

println(EmbedBuilder() == EmbedBuilder()) // true
println(EmbedBuilder().apply { title = "title" } == EmbedBuilder().apply { title = "title" }) // true
println(EmbedBuilder().apply { title = "title"; description = "description" } == EmbedBuilder().apply { title = "title" }) // false

println(EmbedBuilder().hashCode()) // 1
println(EmbedBuilder().hashCode()) // 1

The result makes more sense

@Distractic Distractic marked this pull request as ready for review August 15, 2024 17:23
@lukellmann
Copy link
Member

As already discussed on Discord I don't really feel like this is the right solution. Kord's builder classes are intended to be short-lived, mutable (!) objects that help to create immutable requests and closely model fields of the Discord API (e.g. optional vs. nullable).

Having a data model specific to your domain logic seems to be a way more robust solution to your usecase.

I'm also not aware that implementing equals and hashCode is frequently done for similar builder classes in other projects.

@Distractic
Copy link
Contributor Author

Understood. I thought that could be useful for several projects (like Kordex, I think they keep the builders to compare in a next time) but for my need, I can create my own builders, that allows me to have more flexibility than using Kord builder

@lukellmann
Copy link
Member

Nevertheless, thank you for your work :)

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