> On Dec 7, 2016, at 3:44 AM, Denis Kudriashov <[email protected]> wrote:
>
>
> 2016-12-06 19:29 GMT+01:00 John Brant <[email protected]>:
> > We could make it configurable. "Manually" accessors will be generated in
> > simplified mode and related refactorings will use intelligent mode
>
> My personal preference would be to ask the user what to do when performing
> the accessor refactoring. If the class hierarchy already has a method with
> the name of the variable, we can look at that method and determine what to
> do. In the case of #name, we could see that it is defined in a superclass and
> ask if it is ok to override the method (likely a good choice, but not always
> so it is good to confirm with the user). If the class directly defines the
> method, and that method has a return that returns the variable, then we can
> ask if they want to use that method as the “accessor” even though it does
> some other stuff (and potential returns a different value). Finally, if some
> subclass defines the method, then we can ask if they want to create that
> method even though some subclass is overriding it. Of course, we could have
> any combination of all three too. If they want a different method, then we
> should ask what to name the method instead of appending a number to the name.
>
> Ok. You prefer tons of questions to user just to perform stupid accessors
> generation. And, please, don't say that it is rare cases. It is quite common
> which nicely shown here 18880.
> I wonder do you have real experience when it protected you from destroying
> system? We only talk about simple accessors refactoring.
Yes, I would prefer a question to doing something wrong and breaking my code.
Part of the problem appears to be what is defined as an "accessor”. Accessors
as defined for refactoring are simple get/set accessors (the set version can
return or not return the set value — depending on usage). Using this definition
of accessor we can prove things about the code. However, when you extend the
“accessor” definition then it gets much tougher to prove anything.
Which of these would you consider valid setter methods for foo?
foo: anInteger
(anInteger between: 1 and: 100) ifTrue: [foo := anInteger]
foo: anInteger
(anInteger between: 1 and: 100)
ifTrue: [foo := anInteger]
ifFalse: [self error: ‘Invalid value’]
foo: anInteger
(anInteger between: 1 and: 100)
ifTrue: [foo := anInteger]
ifFalse: [bar := anInteger]
foo: anInteger
foo := anInteger.
self changed
foo: anInteger
self aboutToChange: #foo.
foo := anInteger.
self changed: #foo
foo: anInteger
self change: #foo to: anInteger
foo: anInteger
“do nothing”
foo: anInteger
bar := anInteger
bar: anInteger
foo := anInteger
As for whether things are “rare” or not, that would depend on the usage and the
person performing the refactoring. If you are in the habit of creating some
accessor like methods and then doing a create accessors refactoring, then you
will get this behavior frequently. However, if you are defining the class and
then immediately creating the accessors, then it will be much less frequent.
As for my “real experience”, I wrote the accessor refactoring over 22 years
ago. Since then, I’ve had several times that it has saved me from overriding a
method that I didn’t want to be overridden. As for saving me from destroying
the system, it most likely has, but not in the way you probably expect. When I
would test the refactorings, I would purposely try to destroy the system by
doing dangerous things. If the system crashed, I would fix the refactorings, or
add an explanation that the refactoring didn’t/couldn’t check for that
behavior. Testing in this way gave us confidence when giving demos. We could
allow people to yell out refactorings that they wanted to see performed and we
were fairly confident that everything would keep running. Over the years, we
renamed Object to Thing, True/False to False/True, #+ to #p:, extracted/inlined
bunches of code in String and OrderedCollection, etc. Only once in all of the
demos did we destroy the system. We renamed a method that was being used by the
process scheduler. We don’t try to fix running code with the new names so when
the original method was removed, the process scheduler threw an error and
crashed the image. We could have had the rename method refactoring handle this
case also, but that would have required some non-portable code.
John Brant
