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

[TIMOB-24684] Expose access to interface methods #269

Merged
merged 2 commits into from Feb 2, 2018

Conversation

janvennemann
Copy link
Contributor

overloads = overloads.filter(function (each) {
return each.instance;
});
// TODO Skip overriding methods (i.e. equals(), toString(), etc)? Right now metadata doesn't contain info on that...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So one thing I haven't handled here is that technically interfaces can extend other interfaces, much as classes can extend others. We might want to hook up the JS superclass stuff in here as we do in class.ejs as well. And if an interface doesn't explicitly extend another interface, it really can be set to extend the java.lang.Object type, which would get us the toString()/equals()/hashCode(). (See class.ejs line 64+)

Not sure if I hook up Object as an explicit super class for class.ejs either. I mean outside of Object itself, everything should extend something...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since interfaces allow multi inheritance the simple superclass solution from the class template didn't quite fit. Instead i extended the metabase generator to add all inherited methods from implemented interfaces.

I also removed to manual toString() methods as every object in Java needs to inherit from at least java.lang.Object and gets the toString() method from there.

@@ -201,6 +201,47 @@ private static void asJSON(JavaClass javaClass, JSONWriter writer)
writer.key("methods");
JSONObject methodsJSON = new JSONObject();
Method methods[] = javaClass.getMethods();
generateMethodsMetadata(methods, methodsJSON);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a huge fan of passing in objects to get modified as a side effect of calling a method. I strongly prefer a method returns a new value that we assign/concat onto some collecting object. But this does work for now, so...


// properties
writer.key("properties");
JSONObject propertiesJSON = new JSONObject();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also extract the generateion of propertiesJSON to a method.

<%
} else {
-%>
<%= sanitizedName %>.toString = function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a note about removing the toString() override. I wasn't really sure what was best - to have the Java toString() method "bleed through" to JS or to show a simpler more JS style string.


<%= sanitizedName %>.prototype.toString = function() {
if (!this._hasPointer) return null;
<% if (classDefinition.superClass) { -%>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're basically squashing interfaces down flat (copying the methods from interfaces this one implements) what then is the super class? In that case isn't it always java.lang.Object?

@sgtcoolguy sgtcoolguy merged commit 5191e33 into tidev:master Feb 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants