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-25554] Use correct framework header includes #264

Merged
merged 1 commit into from Nov 27, 2017

Conversation

janvennemann
Copy link
Contributor

@@ -840,7 +840,7 @@ HyperloopiOSBuilder.prototype.generateStubs = function generateStubs(callback) {
return callback(err);
}

var codeGenerator = new hm.generate.CodeGenerator(sourceSet, this.metabase, this.parserState, modules, this.references);
var codeGenerator = new hm.generate.CodeGenerator(sourceSet, modules, this);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like the fact to pass the whole instance to the code-generator. Can we find a different solution? Looks like we don't need all of it's properties. If you pass this.frameworks as a parameter, you should be fine. I know long signatures are also bad in JS, but not sure what's worse :-).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Passing around objects in JS as arguments is always done by reference so this does not have any negative impact on performance.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, but it may be unclear for the clarity of used arguments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, you just have to look into to the constructor if you want to know which properties are used exactly ;) I prefer to pass the object as a whole and let the constructor pick which properties to use. Another benefit, apart from that we don't have to pass 6 parameters around, is that this will also prevent further signature changes should we need another property from the iOS builder in the future.

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 fine then!

@hansemannn hansemannn merged commit 5fd3706 into tidev:master Nov 27, 2017
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