Discordia icon indicating copy to clipboard operation
Discordia copied to clipboard

Iterable classes can be simplified

Open SinisterRectus opened this issue 5 years ago • 3 comments

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 avatar Nov 22 '19 19:11 SinisterRectus

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 avatar Nov 25 '19 22:11 SinisterRectus

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 avatar Nov 26 '19 21:11 SinisterRectus

In 3.0:

  • All cache classes have been reduced to Cache and CompoundCache classes. They are independent from and do not inherit from Iterable and they are only used internally. Thus, Cache, WeakCache, and SecondaryCache are effectively removed from concern.

  • All iterable classes have been reduced to one Iterable utility class.

  • Iterables are initialized from a raw "array-like" table or sequence. In the interest of performance, this table is NOT copied.

  • Objects cannot be added to or removed from iterables after they are initialized except via the original source table.

  • Discordia will either never edit the original source table (as in HTTP requests or gateway events) or will manually make a copy (as in fetching objects from cache). Thus, Discordia's iterables are effectively static.

  • If you are making your own iterables, you should be aware that any changes to the original source table will propagate to the iterable's internal reference to this table.

  • Iterables may be initialized with a key name. If a key name is set, objects may be accessed via that key. For example, a guild iterable may have its key set to "id" and iterable:get("81384788765712384") may be used.

  • Iterables may be initialized with a sorter function and a sorter function may be later set using the sort method. When a sorter function is used in either case, the objects will be sorted using table.sort.

  • Because an array-like table is used, the order of objects is preserved, and iterables may be numerically indexed; thus, iterable:get(1) will return the first object (if one exists).

  • Iterables have a __ipairs method that iterates i, v pairs in their original order.

  • Iterables have a __pairs method that, if a key is set, iterates k, v pairs in their original order; otherwise, __ipairs is used as a fallback.

  • Iterables have a toArray method that returns a raw array-like table of i, v pairs in their original order.

  • Iterables have a toTable method that, if a key is set, returns a raw table of k, v pairs in their original order; otherwise toArray is used as a fallback.

  • Iterable objects may be iterated by its classic iter method, omitting any indices or keys.

  • Iterables may be copied either wholly with the copy method or partially with the filter method. Copies of an iterable will use the same key and sorter of the original, if any are set.

  • The internal data structure of iterables and the (non-)copy behavior may change for optimization purposes.

SinisterRectus avatar Jul 06 '21 16:07 SinisterRectus