MODiX icon indicating copy to clipboard operation
MODiX copied to clipboard

Are Discord.Net.Abstractions necessary

Open patrickklaeren opened this issue 5 years ago • 5 comments

There's a little overhead when Discord.NET's API changes and we need to update the Discord.Net.Abstractions library as part of MODiX to match the new contract(s).

I understand why the abstractions are there, but I'm wondering if there isn't an alternative.

In another bot I am currently authoring, I have "solved" this issue of not being able to test classes which interact with Discord.NET by abstracting D.NET into a service, which deals ONLY with the Discord sides of things and I only implement what I actually require, i.e. https://github.com/Inzanit/d2m/blob/master/src/D2M.Bot/Services/DiscordGuildService.cs

This would take away the overhead of having to update all abstractions which aren't even necessarily used but still provide us with a way of testing.

patrickklaeren avatar Apr 24 '20 13:04 patrickklaeren

cc: @JakenVeina

I'm not against this idea. It's become pretty burdensome to have to update the abstractions library every time Discord.Net changes their APIs (including when they change APIs that we neither use nor care about). As long as we can still maintain the level of testability that we desire, I'd be willing to try this out.

Scott-Caldwell avatar Apr 24 '20 14:04 Scott-Caldwell

To which the obvious argument is "don't change the D.NET version unless there's something in it we need". I would also argue that abstracting through services, rather than through Domain-objects doesn't really change the equation. It's still work to update for any breaking changes in their API.

However, I'm also not against exploring this. There's no good solution for tacking on abstractions to such a large library that doesn't natively support it. I just this think is the least-bad because it's a "true" abstraction. The amount of logic in the abstraction layer is kept to an absolute minimum.

Feel free to throw up a concept.

JakenVeina avatar Apr 25 '20 04:04 JakenVeina

I'd assumed the link I attached in the original description would serve as a pseudoish example.

I'd rather not turn down the option of making things a little simpler in favour of not updating the library. There are often updates which come with nicetities that just bring along API changes.

patrickklaeren avatar May 01 '20 16:05 patrickklaeren

Ah, I see.

A MODiX implementation would have to parameterize more things, mainly guildId as a parameter to pretty much every method, but that would work.

Alternatively, we could just trim back the abstractions library to ONLY include abstractions we actually USE.

I don't really have strong feelings either way. I'll support whatever, so long as it makes our business and UI layers testable.

JakenVeina avatar May 02 '20 01:05 JakenVeina

I'm a little late, but you should absolutely be running the latest (stable) version of Discord.Net whenever possible, as Discord regularly ~~breaks~~ introduces changes into their API. Running an older version might get you into some weird situations where Discord expects one thing, but we send another.

FiniteReality avatar May 02 '20 01:05 FiniteReality