Skip to content

[RFC] Refactoring lib to use "managers" #10

@selenated

Description

@selenated

Proposal: Refactoring the library to use "managers" instead of top-level functions

Background / Reasoning

Right now, the library is set up so that the main index file has all of the functions for the library. This is convenient for consumers, but it's not exactly ideal to keep using one file that can potentially become infinitely long as features are added to the API. While it's handy to have everything at the top-level, keeping it this way can make it difficult to find

Solution

Instead of keeping everything to one file, we're thinking about splitting up model-based functions into "managers", similar to Discord.js. This would effectively move all the functions into namespaced objects like API.systems

Benefits

Aside from cleaning up the index file, it would also give us the opportunity to allow things like built-in caching (for users who opt in) to potentially avoid unnecessary API calls. This would better reflect the structure of the API itself (eg. reflecting /systems/:sid) as well

Drawbacks

This would be a massively breaking change, as none of the functions would be top-level anymore. This also means a bit more overhead and overall code, and could cause issues for references to things like system.members (which would most likely become a manager for fetching members rather than the map of members that it represents now)

Alternative solutions/options

Using Object.assign or mixins

This method is more complicated, but would give us the ability to handle the split while still keeping methods top-level. The only issue is that typing info is a bit weird with our testing and doesn't always seem to carry over properly; plus, there are a good handful of compiler errors that happen due to needing to reference other methods and properties on the API class (not to mention circular dependencies that are a problem). Funny enough, this would be a lot easier if we didn't bother with using TypeScript, but we'd rather keep that for now if possible

Keep top-level fetch methods, make other methods part of their respective classes

In this case we would keep things like getSystem and getMembers at the top level, but would move functions like patchSystem and createMember to the System class. This would significantly clean up the main class, but would have a similar issue to managers: a lot of methods would no longer be top-level and would require at least one extra API call before accessing those methods (eg. you'd need to run getSystem to then use createMember). This would also still make the main class potentially infinite, but it would significantly help with the current amount of methods it has

Just keep everything the same

This is always an option! We don't have to change anything. We're mainly just trying to think long-term, especially since some more complex features may be coming to the API (like fields and display name templates) which could increase the size of the index file significantly. We're not opposed to continuing as things are, though


Any other solutions and ideas are much appreciated! Please feel free to share any thoughts as well, whether it's for or against the main proposal

Metadata

Metadata

Assignees

No one assigned

    Labels

    questionFurther information is requested

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions