PvPAssistant

PvPAssistant

5.4k Downloads

My thoughts about the current state of the addon and reviewing parts of the code.

iam3yal opened this issue ยท 2 comments

commented

So I do think the idea behind the addon is great and once released is going to be tremendously useful to the PvP community and as a PvP player myself I'd love to see it flourish! So everything I'll say is not meant to diminish one's efforts, on the contrary it meant to motivate you guys to do better so there are few engineering challenges that I'd like to share, I'm going to say things with a cold heart and a cold mind in the hope that it will be picked up and improved.

  1. Currently the addon is making use of two libs GGUI and GUTIL and I believe that anything you do especially when you want to build strong foundations you should first avoid introducing big abstractions, big abstractions tend to create a barrier for other developers and many times they bring a baggage that shouldn't be maintained so it's better to start with zero abstractions and once you reach about 80% where the abstraction fits the bill then you can migrate to that abstraction so to make a long story short it needs to be justified and last but not least, abstractions should be as small as possible! this is where building modular and zero cost abstractions are extremely important as opposed to monolithic abstractions or god-like abstractions.

  2. Downloading a tag branch of the addon should be functional! and include all of the dependencies exactly as if you downloaded it straight from curse or any other addons hosting website this is done by introducing a .pkgmeta and make use of externals atm the packager has no clue about dependencies, I see what you do there but imo this isn't ideal.

  3. The addon needs to be LOD. (Load On Demand) and only load when you're in the arena, when you use one of the addon's chat commands, when you open the PVEFrameTab2, ConquestFrame, etc... so it would be resource friendly.

  4. The DataCollection that as far as I can tell is the engine of the addon and should be bug free and work independently from anything essentially as blackbox where data can be queried through events as opposed to retrieved through function calls.

  5. There are many lookups that can be avoided throughout the code and in fact this isn't just an optimization but it's a lot more readable so right now the code is hard to read and easy to follow whereas it should be easy to read and easy follow so you don't have to scan though multiple levels of keys; that's the definition of readable code.

  6. Last but not least it should be at least built with SoC, SRP and ISP principles to make it easier to maintain.

Now, I'm not telling you to rewrite anything or even take any action but I think that some of my points can help reduce some of the pressure in developing this addon and make the codebase quality a lot better so it would be a lot easier to fix existing issues and add new features.

We can if you want talk through some of these points and see together how we can move forward with it.

commented

Hi thank you for that input!
I just glanced real quick over it cause no time today :)
Tmr I will ponder and write my thoughts on this!

commented