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

Document metaclasses and static methods #1117

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

PureFox48
Copy link
Contributor

Thought I'd have a go at this which is currently marked as a TODO.

Thought I'd have a go at this which is currently marked as a TODO.
@mhermier
Copy link
Contributor

The wording about the fact that "class is strange/unusual" is bad. Instead the documentation should extract:

wren/src/vm/wren_core.c

Lines 1276 to 1296 in c2a75f1

// The core class diagram ends up looking like this, where single lines point
// to a class's superclass, and double lines point to its metaclass:
//
// .------------------------------------. .====.
// | .---------------. | # #
// v | v | v #
// .---------. .-------------------. .-------. #
// | Object |==>| Object metaclass |==>| Class |=="
// '---------' '-------------------' '-------'
// ^ ^ ^ ^ ^
// | .--------------' # | #
// | | # | #
// .---------. .-------------------. # | # -.
// | Base |==>| Base metaclass |======" | # |
// '---------' '-------------------' | # |
// ^ | # |
// | .------------------' # | Example classes
// | | # |
// .---------. .-------------------. # |
// | Derived |==>| Derived metaclass |==========" |
// '---------' '-------------------' -'

in some form, and this should be referenced instead, to justify the behavior.

@PureFox48
Copy link
Contributor Author

Well, 'Class' is an unusual class in that it has no metaclass (the only one that doesn't) and I think people will regard it as strange that it's an instance of itself - a bit reminiscent of the Russell paradox.

This isn't intended as a criticism as I can understand why it's been done this way and object hierarchies of any programming language tend to have a certain amount of strangeness.

Personally, I'm not in favor of including the object hierarchy diagram in what is intended to be simple, readable documentation and I don't think it highlights the strangeness of the 'Class' class very well in any case. The examples I've chosen hopefully do that.

@mhermier
Copy link
Contributor

Well I think, the diagram should belong to the documentation, for advanced explanations of how the object model is implemented. Referring it from that change would seems logical, and would would allow some "not-personal" comments like:

System.print(Class is Class) // true, look here for details

@PureFox48
Copy link
Contributor Author

Well, if we were to include a link to the diagram in the Class docs, then the best place to do it would be in the 'Inheritance' section which needs looking at anyway as it doesn't integrate too well with the 'Super' section at present.

I'm still not keen on referencing it in the 'Metaclasses' section (I can't include a clickable link to it in a code comment anyway) as I don't think the last two examples are easy to understand without a written explanation.

@PureFox48
Copy link
Contributor Author

OK, I've changed the 'Metaclasses' section so that it no longer refers to 'Class' as being unusual or strange.

@PureFox48
Copy link
Contributor Author

I've added some wording to the 'Static methods' sub-section to make it clear that static getters, setters and operators are supported and that, whilst this can be used in a static method, it refers to the class object itself rather than an instance of it.

@PureFox48
Copy link
Contributor Author

There is one remaining TODO in the Classes docs, namely better integration of the 'Super' section, and as I don't think this needs major surgery, I thought I'd include my attempt at it within this PR which I've now done.

As suggested earlier by @mhermier, I've included a link to the object hierarchy diagram.

Missing word in Static methods section.
@PureFox48
Copy link
Contributor Author

Reading through the whole lot again, I noticed that I'd missed out a word in the 'static methods' section. Fixed now.

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

Successfully merging this pull request may close these issues.

2 participants