On Wed, Dec 7, 2016 at 9:00 AM, John Brant <[email protected]> wrote:
> > 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 > Amen. If it ain't broke, don't fix it. It should go without saying that if it ain't broke, don't break it. Changing the accessor refactoring is breaking things. _,,,^..^,,,_ best, Eliot
