Skip to content

Plat 928 #3

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

Merged
merged 17 commits into from
May 4, 2016
Merged

Plat 928 #3

merged 17 commits into from
May 4, 2016

Conversation

gianchub
Copy link
Owner

No description provided.

@mattbennett
Copy link
Collaborator

This is beautiful.

Why did you use name mangling in the IgnoresManager?

@gianchub
Copy link
Owner Author

Thank you sir.

Both ignore and tables are mutable, and they percolate quite deep in the compare chain of calls, so I thought, thinking about a suggestion you gave me in my first week at student.com, if someone wants to use this object let's try not to expose it to unnecessary risks. So I wrote the properties to make it more explicit that that is the way to get access to the whole objects, made them return copies, and used name mangling to reinforce the whole thing.

To be honest, if you don't think it's appropriate I can change it, it's not essential at all for the functioning of the whole object. I could just as easily use a single underscore just to suggest they're private.

Let me know what you think :)

@mattbennett
Copy link
Collaborator

👍

@mattbennett mattbennett merged commit 63fe8b9 into PLAT-818 May 4, 2016
@gianchub gianchub deleted the PLAT-928 branch May 4, 2016 15:06
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