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

ActiveHash instance should not be editable #146

Closed
syguer opened this issue Apr 1, 2017 · 3 comments
Closed

ActiveHash instance should not be editable #146

syguer opened this issue Apr 1, 2017 · 3 comments

Comments

@syguer
Copy link
Collaborator

syguer commented Apr 1, 2017

Relating #125
ActiveHash focus on master data. So I think we should drop( or prohibit) editing(update/destroy) on version 2.0.0.
(but, it is easy to change instance value on Ruby. I mean we should drop it from open API)

I know it is very aggressive change so I want to hear your opinion.

@stcorbett
Copy link

hmm... Instance variables are core functionality for ruby objects. They are less known or used in rails-land, I'd suspect a rails app that modifies them during a request probably confuses more rails users than it helps...

Having a strict and clear "don't use instance variables on these objects" policy sounds better than an unclear policy and unexpected behavior which is more of what is in place at the moment.

Is there an straightforward way to "turn them off" for ActiveHash objects?
Do you know of other ruby/rails objects that restrict instance variable functionality?

@syguer
Copy link
Collaborator Author

syguer commented Apr 4, 2017

I think it is OK to use instance variables, problem is not to be valid access permission.
Encapsulation is base theory of Object-Orientated programming. Ruby object can access to instance variables doesn't through accessor(like #set_instance_valiables). Able to do is not always OK to do, isn't it?

Having a strict and clear "don't use instance variables on these objects" policy sounds better than an unclear policy and unexpected behavior which is more of what is in place at the moment.

Sounds good. we should do.

I closed your PR once. But I'm going to reopen & merge if we have no idea for strict value of ActiveHash 🤔

@syguer
Copy link
Collaborator Author

syguer commented Apr 5, 2017

Oh sorry, our discussion point was not matching, and I finally understand.
A things you want is finder should not return original instance. Right?
It seems to be unrelated to instance variables. I'm OK to fix it.

Let's taking about this issue on #124

@syguer syguer closed this as completed Jul 18, 2019
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

No branches or pull requests

2 participants