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

Issues with class [wordy] #142

Open
andrewstarks opened this issue Apr 8, 2015 · 3 comments
Open

Issues with class [wordy] #142

andrewstarks opened this issue Apr 8, 2015 · 3 comments

Comments

@andrewstarks
Copy link
Contributor

Steve,

I keep trying to love pl.class, but i think it still needs some help. Of course, code beats complaining and I did code fixes for my issues. However, every time I submit a pull request, you merge it! I am really not comfortable with my work-arounds, and so no pull request until you've looked at this. However, my fork is here, if you want to look at it:

https://github.com/trms/Penlight.git
[trms branch]

Insufficient documentation.

  • _class_init: something that a base class does to the classes that inherits from it? Like "make fancy getter or setter" in class.properties?
  • _post_init: a base class method that is run post-_init of the inherited class?
  • _create: a function that is run on the arguments received. I believe this is intended to be a sort of default argument processor?
  • _handler: a way to signal pl.class that the index method was made special and needs to be inherited?

Also, does all of this need to be in existence? If so, how do they work together and how don't they work together? If you're using these in pl, then they should be documented for use by us mortals.

properties and catch don't work together

AKA #101

properties does a plain t[k] lookup to see if it's a simple class lookup. This triggers catch, except catch receives the class and not the table as self. Also, it's most likely backwards of what you want, which is to do the normal thing and then catch. This is fixed in my branch. I also added _setter to compliment _handler. I'm having trouble conceptualizing the difference between _handler and catch, btw.

_ENV / _G Pollution

Putting the classes into the local _ENV makes it behave like Lua has real classes (types), but it isn't very helpful for introspection/serialization/cross-thread-shenanigans. That is, I receive a table and it has a type name. I can do _ENV[my_type_name_field], but this seems unwise.

In my branch, I added a _PL_USE_GLOBALS flag, which defaults to the current behavior. When it is false it stores the classes in class.classes. Also, class.foo_class returns an existing class, if it is there, instead of making a new class. If you want to remove a class, then class.classes.foo_class = nil does the trick.

Generally, I'd argue for more breakage of compatibility in favor of fewer legacy options. One way to do things beats two ways, especially considering that defining "one way" is one of the driving motivations for picking something like pl. As is, pl.classes is somewhat smelly, in that regard.

I have a new slogan: "Penlight: Policies for Lua"

But, you still should follow PUC/Rio's lead and anger everyone with breaking changes, now and again.

Also, please remember that if we're not complaining, you're not relevant. ;)

@stevedonovan
Copy link
Contributor

Getting back into bringing PL up to date now, and I like the idea of adding more extensibility to the class system, always with the proviso that the plain case remains fast and simple. Part of that is indeed documenting (and maybe rethinking) shenanigans. I like the idea of letting users decide whether they want global pollution - the trouble with this notation is that it does two things: one is give the dog a name, and the other is make the dog globally available (in some sense). I suppose for me it's like the require 'pl' trick - not recommended for formal code like writing modules - except classes having names is useful for debugging.

So the revamp has started - I will look at your fork.

@stevedonovan
Copy link
Contributor

Thanks for reminding me that I'm not irrelevant, Andrew!

@andrewstarks
Copy link
Contributor Author

Ha!

Don't look at my repo. I'm sure it's buggy and half-done. I don't use that anymore. I use my_project_name.pl to load penlight and then I wrap the class library in some magic.

That said, I'm not even 100% confident in my magic, although it hasn't been a problem. It's just that I wouldn't vouch for its value.

And that's really the thing: value. As is, I think the class library may benefit from a depreciation and rebirth. A re-asking of the basic questions:

1: What problem does it solve?
2: What limitations does it need?
3: Who is it not for?
4: ?

This may be too re-inventive. I may be colored by my unguided (and uninformed) attempts. I think i'm opinionating that this might be more of a conversation than an issue.

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

No branches or pull requests

2 participants