Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Class Commons "2.0" #5

Open
bartbes opened this issue May 20, 2015 · 8 comments
Open

Class Commons "2.0" #5

bartbes opened this issue May 20, 2015 · 8 comments

Comments

@bartbes
Copy link
Owner

bartbes commented May 20, 2015

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.

@gvx
Copy link
Collaborator

gvx commented May 20, 2015

I both love and hate this idea!

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

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. 😠

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

@bartbes
Copy link
Owner Author

bartbes commented Jul 9, 2015

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

@vrld
Copy link
Collaborator

vrld commented Jul 10, 2015

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.

@tst2005
Copy link
Contributor

tst2005 commented Jul 27, 2015

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,

@bartbes
Copy link
Owner Author

bartbes commented Jul 28, 2015

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.

@tst2005
Copy link
Contributor

tst2005 commented Jul 29, 2015

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,

@bartbes
Copy link
Owner Author

bartbes commented Jul 29, 2015

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!

@tst2005
Copy link
Contributor

tst2005 commented Jul 31, 2015

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,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants