SinisterRectus / Discordia

Discord API library written in Lua for the Luvit runtime environment
MIT License
711 stars 145 forks source link

Iterable classes can be simplified #222

Open SinisterRectus opened 5 years ago

SinisterRectus commented 5 years ago

One of the defining features of Discordia is its inclusion of an object-oriented interface for interacting with major Discord types. "Containers" provide an interface for interacting with a single instance of a Discord type while "Iterables" provide an interface for interacting with sets of multiple Containers. This is in contrast to providing only raw tables.

For example, iterable objects provide methods to replace the regular behavior of Lua tables. While the end products in these examples are equivalent, the methods can hide implementation details that are not important to the user, but may be important to the operation of the library.

For example:

local guild = client.guilds["1234567890"]

vs

local guild = client.guilds:get("1234567890")

In addition to this encapsulation, methods exist to make certain common operations a bit easier.

For example:

local n = 0
for _, emoji in pairs(guild.emojis) do
  if emoji.animated then
    n = n + 1
  end
end

vs

local n = guild.emojis:count(function(e) return e.animated end)

The flaw here, I think, is that in an attempt to encapsulate different internal Discordia behaviors, I made this interface unnecessarily complicated. We have Cache, WeakCache, SecondaryCache, ArrayIterable, TableIterable, and FilteredIterable classes. There are reasons for why each class exists, and the fact that they do exist may provide hints to the user about the appropriate use of each one, but if they all share the same methods, is worth it to have them all? I will provide more thoughts on this below later.

SinisterRectus commented 5 years ago

Cache

The OG workhorse.

Discord expects users to maintain an active WSS gateway connection in addition to using its HTTPS API, both of which typically provide full sets of data for objects. It is the client's job to apply this data to any already-existing cached objects or create new objects if old ones do not exist. To facilitate this, Discordia has a Cache class that can be fed data, update or create an object, and return the object without the caller having to worry about whether it previously existed.

In retrospect, this class probably should have been an internal one, as it was in Discordia 1.x, and as is suggested in #142. All of its unique methods are private methods and all of its public methods are not unique. From a user's perspective, the class has no good reason to exist, but it still is useful for developing and maintaining the library.

WeakCache

More like weak class.

The purpose of a WeakCache is to provide a version of Cache that lazily discards its objects after some time. Used only for message caching and audit log webhooks. For more detail about the implementation of this class, see #214. In addition to the problems listed there, WeakCache also shares the problems described above for Cache.

Since the introduction of the _deleted weak table in the Cache class with commit 4eefacad005903f6768654f0fbd53da9a7535e0f, WeakCache, as it is currently implemented, is now redundant. To weakly cache an object, one could use a Cache where objects are directly inserted into the _deleted table, bypassing the _objects table.

SecondaryCache

The unsung hero.

What this class does is provide an invisible wrapper around a regular Cache such that raw Discord data can be inserted or removed into it without it being the primary storage location for those objects. For example, when TextChannel:getMessages is called, message data is inserted into a SecondaryCache, which uses the TextChannel.messages cache to create or update each Discordia object. Like the Cache that it wraps, this class is useful for developing and maintaining the library, but its existence is not important to the library user.

SinisterRectus commented 4 years ago

ArrayIterable

Arrays start at what?

This class internally wraps a Lua sequence with iterable methods and with a map function that is called on each element when it is accessed. In practice, this is always a sequence of Snowflake IDs where the map function returns the result of querying a cache or caches. For example, Member.roles stores a sequence of role IDs, but calls Member.guild.roles:get(id) to provide a role object instead of just the ID.

TableIterable

Everything is a table.

This class behaves identically to ArrayIterable. The only difference is that the table that it internally wraps is a hashed table and not a sequence. In practice, this is only used for GuildVoiceChannel.connectMembers to iterate over voice states, which are hashed by user ID.

Because this class serves a similar purpose to ArrayIterable and because its use is rare, it may be reasonable to combine both into one class. There would be a cost to doing this, but it's not yet clear what that cost is.

FilteredIterable

Now you're just making things up.

If I remember correctly, this class was somewhat of an afterthought and added late in the development of Discordia 2.0.0. Instead of wrapping a raw Lua table like ArrayIterable and TableIterable do, this class wraps any other Iterable subclass and only accesses it with the findAll method to provide a subset of that wrapped Iterable. For example, Role.members wraps Role.guild.members and uses Member:hasRole to provide only the members that have the target role. Accessing objects this way is a relatively expensive operation, since it's basically doubling up on every operation, but this is convenient since it allows the wrapped object to update freely without having to propagate its changes to the wrapper.

SinisterRectus commented 3 years ago

In 3.0: