-
Notifications
You must be signed in to change notification settings - Fork 8
Remove duplicate comment from methods #55
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
Conversation
export class ExtendedPythonGenerator extends PythonGenerator { | ||
private currentModule: commonStorage.Module | null = null; | ||
private mapWorkspaceIdToExportedBlocks: { [key: string]: Block[] } = Object.create(null); | ||
protected methods_: {[key: string]: string} = Object.create(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why protected instead of private?
Why the trailing _?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I almost always go for protected instead of private so it can be subclassed later.
The trailing _ was because I was copying the style from definitions_
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm the opposite. I go for the most restrictive visibility first and then make it more visible only if necessary.
If it needs to be subclassed later, it can be changed from private to protected then.
https://google.github.io/styleguide/tsguide.html#visibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I agree fully about not making things public by default (as the style guide says), protected seems to be better than private. Imagine the mess we would be in if Blockly had made everything private instead of protected. (I'll admit that this tendency of mine is because I am almost always creating libraries or systems instead of applications.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense for libraries where you (as author of the library) want to allow a class to be subclassed. In this case, it is not a library and within this application, we don't currently want to subclass ExtendedPythonGenerator.
I'm going to approve (and merge) this PR so we can keep making progress.
Fixes #25