Skip to content

refactor: Common classes/interfaces for common classes code and better pattern matching #1061

Draft
MayaChen350 wants to merge 9 commits intokordlib:mainfrom
MayaChen350:main
Draft

refactor: Common classes/interfaces for common classes code and better pattern matching #1061
MayaChen350 wants to merge 9 commits intokordlib:mainfrom
MayaChen350:main

Conversation

@MayaChen350
Copy link
Copy Markdown
Contributor

Following my PR at #1011, I decided to check what else could also benefit from some common interfaces or classes, sealed or not

I also added some tests to make sure the changes will compile on older projects

However, I might be wrong on some stuff, so reviewing would be appreciated!!

Also please consider: modifying sealed classes after they're merged might break some projects (if they're using exhaustive branches) so discussing them first (also to see if they can possibly be sealed or not) would be essential 🙏🙏

If you have any ideas or things you would like me to add or change please tell!

@MayaChen350
Copy link
Copy Markdown
Contributor Author

MayaChen350 commented Mar 29, 2026

(I'm not done yet but should I run upgradeLegacyAbi before every commit? Thanks)

@MayaChen350
Copy link
Copy Markdown
Contributor Author

MayaChen350 commented Mar 29, 2026

uhh I'm not sure why it's crashing it builds for me

Edit: Nevermind maybe it's because I have some uncommited changes locally

I wasn't sure what to do with MessageBulkDeleteEvent so I made two
interfaces to attempt both my ideas
@MayaChen350 MayaChen350 changed the title refactor: Common classes/interfaces for better pattern matching refactor: Common classes/interfaces for better pattern matching and code deduplication Mar 29, 2026
@MayaChen350 MayaChen350 changed the title refactor: Common classes/interfaces for better pattern matching and code deduplication refactor: Common classes/interfaces for common classes code and better pattern matching Mar 29, 2026
Not too sure of those methods names though
*
* MIGHT FAIL!
*/
public fun <T> MaybeThreadChannel.flatMap(func: () -> T): Result<T>
Copy link
Copy Markdown
Contributor Author

@MayaChen350 MayaChen350 Mar 29, 2026

Choose a reason for hiding this comment

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

Maybe it could be renamed to just try
Whatever feels the most Kotlin I guess

Copy link
Copy Markdown
Contributor

@NoComment1105 NoComment1105 left a comment

Choose a reason for hiding this comment

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

Sorry for leaving this, I may or may not have forgotten about it :).

I'm not really a fan of some of the names (Maybe... Channels...) of the interfaces used, they don't feel right for the project. Ideally update would be used but I understand that's in use. I don't know if anyone has any better suggestions for names? Otherwise it seems mostly good, but I'd need to do a finer look than my current one

@MayaChen350
Copy link
Copy Markdown
Contributor Author

Sorry for leaving this, I may or may not have forgotten about it :).

I'm not really a fan of some of the names (Maybe... Channels...) of the interfaces used, they don't feel right for the project. Ideally update would be used but I understand that's in use. I don't know if anyone has any better suggestions for names? Otherwise it seems mostly good, but I'd need to do a finer look than my current one

Yeah that's really fair. Honestly anything better than my haskell-sounding (like, Maybe 🥀) (I don't even use Haskell) or those names in plural would definitely be better.

I'm open for suggestions!

@MayaChen350
Copy link
Copy Markdown
Contributor Author

MayaChen350 commented Apr 2, 2026

Otherwise it seems mostly good, but I'd need to do a finer look than my current one

And thanks! There's way more work that could be done though that at this point I wonder if I should put everything in the same PR.

Also another thing I was thinking about... What's actually my goals there...? 😅

I realized while doing it that well... Pattern matching would mainly makes sense if something returns one of those... And those interfaces could be I guess for if there is a need to do an operation on all of those... Though I guess it's a library so maybe people can get original?? But like what's the point of like, something that is either a thread channel or a deleted thread channel if per exemple, it's never going to be returned by the library ever or if you should always be able to guess what it is? (Except if you would store them maybe)

Also I guess deduplication of code is good but at some point it might get messy. Especially that I end up sharing a lot of fields... in interfaces... Maybe I could use some abstract classes or something instead for some if that makes sense too

Also at this point there is so much shared stuff that I'm not sure what I should share with those classes but not with those, like choosing what should inherit of what.

tldr: I'm struggling on how to make those changes actually "useful"

@NoComment1105
Copy link
Copy Markdown
Contributor

Yeah I do see what you're saying a bit. Your previous PR was targetted and definitely helped in reducing code duplication, I wonder if this one in its current state is a bit too general and that it should maybe focus on more specific situations (like reactions in the other PR) and leave the wider things for now?

@MayaChen350
Copy link
Copy Markdown
Contributor Author

That's a good idea I'll think about what to do

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