Class-Commons icon indicating copy to clipboard operation
Class-Commons copied to clipboard

Class Commons "2.0"

Open bartbes opened this issue 9 years ago • 8 comments

Class Commons has been fixed for a while now, and perhaps it's time to shake it up a little, fix our grievances, and clean it up.

I had some ideas regarding a new, cleaner interface. In particular, the ugliest wart of Class Commons, in my opinion, is that it relies on globals. I have devised a way around this, but it also has this wonderfully hacky feel to it, so I welcome all feedback.

If anyone else has ideas, or proposals, please do share, I think we should generally strive to keep the specification as stable as possible.

Without further ado:

  • A class commons implementation inspects package.loaded["class.commons"], if it does not exist it provides it, as if it were the common table from before.
  • A class commons user uses pcall(require, "class.commons") to find its implementation.
  • Local fallback implementations for users are allowed (as before) and encouraged (maybe? that sounds like a lot of duplicate work), but shall remain local to that user. That is, if there is no class commons implementation already available, a library using class commons shall not provide one to anything but itself.

This means we can get rid of both globals, and package.loaded/require are used as direct replacements.

bartbes avatar May 20 '15 17:05 bartbes

I both love and hate this idea!

On the one hand, it's so much less messy than globals. :smiley:

On the other hand, this seems to suggest you could do require("class.commons"), which only works if a compatible class implementation is already required, violating the principle of least surprise. :angry:

On the third hand, I can't think of a better place to hide the commons, so yeah. :+1:

gvx avatar May 20 '15 23:05 gvx

Any further input from @vrld @kikito or @Yonaba?

bartbes avatar Jul 09 '15 15:07 bartbes

I don't have a strong opinion on this. On the one hand, the current spec works and you know what they say about how often to change a running system. On the other hand, hiding class.commons inside package.loaded is more elegant than putting into the global namespace. However, maybe the name should include spaces or other characters to make sure it does not collide with a hypothetical class.common module.

vrld avatar Jul 10 '15 18:07 vrld

Hello guys,

I have a proposal for ClassCommons 2.0. Take a look at https://github.com/tst2005/PrososalForClassCommons2 I done samples for middleclass, SECS, 30log, hump.class, ... and little test suite.

Regards,

tst2005 avatar Jul 27 '15 23:07 tst2005

To keep it all central, I'll comment in this ticket:

  • I'm not a fan of having a "Class Commons" file that people need to include, the point of class commons was that it would just work™ (though whether it was successful is another matter altogether).
  • I'm not sure how your register function would actually do anything, it doesn't change anything about the passed function, and, importantly, doesn't make the class "native", it just stuffs it in a table somewhere.
  • In fact, it seems like you're aiming to support multiple class libraries in the codebase, which is exactly what class commons is trying to prevent.

bartbes avatar Jul 28 '15 09:07 bartbes

Hello Barbes,

At first, I was sad to read your comment. I only see "I'm not a fan", "I'm not sure", "In fact ... is exactly what ... trying to prevent".

But now, I understand my goal seems very different from your one with ClassCommons2. I like my approach to support multiple implementations. I will drop the ClassCommons2 name to avoid conflict and make my own project to manage multiple implementations (probably not only for class system).

Regards,

tst2005 avatar Jul 29 '15 12:07 tst2005

I was worried I was too negative, but I chose to be honest about my concerns, rather than sugarcoat it, and I'm glad you took it well.

It seems your goal doesn't align with Class Commons, but it is definitely an interesting one. Good luck with your project!

bartbes avatar Jul 29 '15 13:07 bartbes

I think there is one common point between ours both goals : using local/module environment instead of global one. To be able to check my own implementation, I changed the Class-Commons-Tests/test.lua to drop global environment use. I also wrote a global environment locking module (gro). The current tests get the require returns table as the common table and try to get the class and instance directly inside it. I hope it should help. Regards,

tst2005 avatar Jul 31 '15 09:07 tst2005