Incapamentum / Exalted-Sage

Discord bot that does API requests to retrieve a collection of daily achievements for the next day and pushes an alert when any that is in a watchlist shows up.
3 stars 0 forks source link

Make better use of OOP paradigm (inheritance, polymorphism, etc) #51

Open Incapamentum opened 1 year ago

Incapamentum commented 1 year ago

One thing I need to start getting used to, and start taking advantage of, is the OOP paradigm. I've been noticing a lot of boilerplate code that could be reduced by creating a parent class and having child classes inherit from it.

One notable example is in the Models. All effectively follow a similar format outside of the way how information is being held. Would be good to take advantage of such a concept.

In addition, looking into generics will also be beneficial.

Incapamentum commented 1 year ago

Another observation: many of the functions in DatabaseService.cs are effectively cookie-cutter. For example:

internal static async Task<Dictionary<int, string>> GetDailyPveAchievements(MongoClient client)
{
    var achievementCollection = GrabCollection<AchievementDoc>(client, "achievements");
    var dailyAchieves = await GrabDocument(achievementCollection, "Daily Achievements");

    return dailyAchieves.Achievements;
}

vs

internal static async Task<List<string>> GetDailyPveWatchlist(MongoClient client)
{
    var watchlistCollection = GrabCollection<WatchlistDoc>(client, "watchlists");
    var watchlistDoc = await GrabDocument(watchlistCollection, "Daily Watchlist");

    return watchlistDoc.Watchlist.ToList();
}

It's all essentially the same structure. Just not sure if it'd be possible to generalize it, perhaps through generics. Will have to look into this.

Incapamentum commented 1 year ago

As noted in #56, along with the prior comment, there definitely needs some refactoring done into the DatabaseService.cs class, but it should be tracked by a separate issue so as to not muddle the purpose of each.

Incapamentum commented 1 year ago

It's possible the scope of trying to refactor things is starting to become large. Mostly cause dealing with how DatabaseService.cs is currently implemented is being a major pain to work into unit tests.

Incapamentum commented 1 year ago

Regarding the comment on the DatabaseService.cs file: it was recently refactored and made stateful (as noted in #59). However, it's possible that templates may be made depending on the return type. For example: a Dictionary<><> template and a List<> template. That should be a good start.

Incapamentum commented 1 year ago

Something I want to point out: a lot of the event handlers pretty much all share similar member variables. A lot of code is being retyped due to this. Definitely will have to refactor this, possibly in the next tock-cycle.

Incapamentum commented 1 year ago

Something I want to point out: a lot of the event handlers pretty much all share similar member variables. A lot of code is being retyped due to this. Definitely will have to refactor this, possibly in the next tock-cycle.

This specific note has been done via #63 and #64.