Could we deselect check box by default in Nautilus if refactoring suggests instVar1 / instVar1? Every time have to go through the list and deselect...
Cheers, Alex On 8 December 2016 at 14:11, Eliot Miranda <eliot.mira...@gmail.com> wrote: > Hi Nicolas, > > On Dec 8, 2016, at 1:16 AM, Nicolas Passerini <npasser...@gmail.com> > wrote: > > Let me recapitulate a little. > A) What to do in case that a method with same selector already exists in > the same class? > So far I think we've come up with four ideas: > 1. Create with an alternative name such as instVar1 / instVar1: > 2. Create with an alternative name but ask the user what the name should > be. > 3. Replace existing method with simple accessor. > 4. Do nothing. > > > To be clear, you don't mean Do Nothing; you mean Issue an Error and Abort > the Refactoring right? > > > I think that the last one is the best alternative and should be the > default action. Let me analyze the others: > 1. Is the current behavior, but I always end up deleting the method, I > can't think of a situation in which I want to keep a method named #name1 . > I know that there was a lot of thinking on top of this ideas, but even so > I think that we can (should) rethink things once in a while. > > 2. A little better, because lets me select a name better than #name1. > Still I do not like it, because this means that I have a Class with > - an instance variable 'name' > - a method #name which is not the accessor for 'name' inst var. > - and an accessor which is named in another way > > I think this can happen in two situations: > - The original #name method is in fact a (smarter) accessor (i.e., for > example it provides a lazy initialization). In this case the best action is > 4 => do nothing, accessor already exists, do not provide a new one. > - The original #name does something else, that maybe has nothing to do > with the variable name... in this case, I think the best is aborting the > refactor. I am not proposing that refactoring browser should abort the > refactor automatically, probably not, but that is what I would do as user: > my class has a weird name clash, so I should rename either the inst var or > the method, it has no automatic solution. > > I am not sure about how the "generate accessors" could help me coping with > name clashes, but I do not like generating accessor with another name > different from the inst var name is a good idea. We could have this as an > option to the user, but should not be the default. > > 3. This is really dangerous, we shouldn't do it. I am not sure if someone > would like to have this as a (non default) option. > > > B) What if the colliding method is in a superclass? > Well, pretty much the same but: > - in option 3 instead of replacing a method you could override it. > - the method in the superclass could not be a "smarter accessor" unless > you are trying to create accessors in subclass for an inst var in the > superclass... I do not like it. So I'd consider it allways as a name clash. > > Then I think that the best option is still do nothing (option 4). You > could offer to create method with another name as alternative (option 2)... > but I still think that this kind of name "coincidences" may introduce bugs > in the future and should be avoided. > > C) Colliding methods in subclasses. > Here we have a more subtle situation because the existing method could be > a smarter accessor, that even could have its motivation to be there (for > example because initialization code only is useful for one subclass). > > I still think that doing nothing could be a safe solution, better than > creating it with an alternative name (but we could allow it also). > > Here we could think also about writing the accessor, because it will not > override preexistent behavior. But we have to think about a few situations: > - Easy, if any of the colliding methods (they could be serveral one for > each subclass) is not a smarter accessor... we have name clashes and we > cannot do anything. > - If all colliding methods are smart accessors, we could create one in the > superclass, provided that there exists at least one subclass which does not > provide its own implementation. > > > Of course there is not precise way of determining if a preexistent method > can be considered as a smarter accessor or not... so we only can let the > user decide. > > To resume: > - Doing nothing will be the best default as it is suitable for all cases, > and can not do any harm. > - Creating methods with alternative names could be optional, but shouldn't > be default. In this case I prefer to ask the user for a name instead of > creating one automatically. > - Creating the method even in case of collision can be dangerous... I am > not sure if we should even allow it as an option. > > 2016-12-07 0:03 GMT+01:00 Ben Coman <b...@openinworld.com>: > >> How about mostly keeping the existing behaviour, but default to >> collisions being deselected? >> Then at least you don't need to search for it. >> It minimises the chance of missing it by mistake. >> The deselected item easily stands out to be reselected if required. >> >> P.S. An additional side idea: If collision is from a superclass then >> append (Superclass>>name) to the list item, and clicking on it shows a >> code dialog to the right similar to Spotter code review pane, to make >> a review easier. >> >> cheers -ben >> >> On Tue, Dec 6, 2016 at 10:09 PM, Yuriy Tymchuk <yuriy.tymc...@me.com> >> wrote: >> > It shouldn’t be too intelligent. Keep the default behavior, and for each >> > method add a 3-state checkbox: create (default), skip, force (override). >> > Also having a usage recorder would be nice to know if developers are >> > actually checking the other state more often than the default one. >> > >> > Uko >> > >> > On 6 Dec 2016, at 13:03, Thierry Goubier <thierry.goub...@gmail.com> >> wrote: >> > >> > >> > >> > 2016-12-06 11:34 GMT+01:00 Denis Kudriashov <dionisi...@gmail.com>: >> >> >> >> >> >> 2016-12-06 11:28 GMT+01:00 Yuriy Tymchuk <yuriy.tymc...@me.com>: >> >>> >> >>> Additionally it was annoying if a superclass has a method with the >> same >> >>> name. For example if you have a name var and you create accessors I’d >> like >> >>> to have actually a ’name’ getter and not ’name1’ >> >> >> >> >> >> Yes >> >> >> > >> > If you start to make it so intelligent it requires half a dozen click to >> > generate a simple accessor, then people will stop using the refactoring >> and >> > write the code by hand... >> > >> > Thierry >> > >> > >> > >> >> >
