Refactoring>checkPreconditions behaves funny about the errorBlock

> Hi,
>
> The RB refactorings do some obscure dance around an errorBlock, which
> is always nil, as far as I can see. As I was trying to supply a
> non-nil error block, I was astonished to find out that even if it
> isn't nil, it won't be executed. Here's the relevant snippets:
>
> checkPreconditions
>        | conditions block |
>        conditions := self preconditions.
>        conditions check
>                ifFalse:
>                        [block := conditions errorBlock.
>                        block notNil
>                                ifTrue: [self refactoringError: conditions errorString with: block]
>                                ifFalse: [self refactoringError: conditions errorString]]
>
> refactoringError: aString with: aBlock
>        ^ RefactoringError signal: aString with: aBlock
>
> I can't imagine how that could help anyone. I suggest either removing
> the errorBlock, or turning it into a good old instance variable, and
> running it in case of an error.
>
> Cheers,
>
> Niko
>
> --
> http://scg.unibe.ch/staff/Schwarz
> twitter.com/nes1983
> Tel: +41 076 235 8683
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

> This is used to validate the preconditions of refactorings and inform
> the user about any mismatches.
>
> It should better not be removed.
>
> Lukas
>
> On 4 October 2010 15:36, Niko Schwarz <[hidden email]> wrote:
>> Hi,
>>
>> The RB refactorings do some obscure dance around an errorBlock, which
>> is always nil, as far as I can see. As I was trying to supply a
>> non-nil error block, I was astonished to find out that even if it
>> isn't nil, it won't be executed. Here's the relevant snippets:
>>
>> checkPreconditions
>>        | conditions block |
>>        conditions := self preconditions.
>>        conditions check
>>                ifFalse:
>>                        [block := conditions errorBlock.
>>                        block notNil
>>                                ifTrue: [self refactoringError: conditions errorString with: block]
>>                                ifFalse: [self refactoringError: conditions errorString]]
>>
>> refactoringError: aString with: aBlock
>>        ^ RefactoringError signal: aString with: aBlock
>>
>> I can't imagine how that could help anyone. I suggest either removing
>> the errorBlock, or turning it into a good old instance variable, and
>> running it in case of an error.
>>
>> Cheers,
>>
>> Niko
>>
>> --
>> http://scg.unibe.ch/staff/Schwarz
>> twitter.com/nes1983
>> Tel: +41 076 235 8683
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>
>
>
> --
> Lukas Renggli
> www.lukas-renggli.ch
>

> See for senders of #errorBlock:, e.g.
> PushDownInstanceVariableRefactoring, RemoveClassRefactoring or
> RemoveInstanceRefactoring.
>
> On 4 October 2010 15:40, Lukas Renggli <[hidden email]> wrote:
>> This is used to validate the preconditions of refactorings and inform
>> the user about any mismatches.
>>
>> It should better not be removed.
>>
>> Lukas
>>
>> On 4 October 2010 15:36, Niko Schwarz <[hidden email]> wrote:
>>> Hi,
>>>
>>> The RB refactorings do some obscure dance around an errorBlock, which
>>> is always nil, as far as I can see. As I was trying to supply a
>>> non-nil error block, I was astonished to find out that even if it
>>> isn't nil, it won't be executed. Here's the relevant snippets:
>>>
>>> checkPreconditions
>>>        | conditions block |
>>>        conditions := self preconditions.
>>>        conditions check
>>>                ifFalse:
>>>                        [block := conditions errorBlock.
>>>                        block notNil
>>>                                ifTrue: [self refactoringError: conditions errorString with: block]
>>>                                ifFalse: [self refactoringError: conditions errorString]]
>>>
>>> refactoringError: aString with: aBlock
>>>        ^ RefactoringError signal: aString with: aBlock
>>>
>>> I can't imagine how that could help anyone. I suggest either removing
>>> the errorBlock, or turning it into a good old instance variable, and
>>> running it in case of an error.
>>>
>>> Cheers,
>>>
>>> Niko
>>>
>>> --
>>> http://scg.unibe.ch/staff/Schwarz
>>> twitter.com/nes1983
>>> Tel: +41 076 235 8683
>>>
>>> _______________________________________________
>>> Pharo-project mailing list
>>> [hidden email]
>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>>
>>
>>
>>
>> --
>> Lukas Renggli
>> www.lukas-renggli.ch
>>
>
>
>
> --
> Lukas Renggli
> www.lukas-renggli.ch
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

> Ok, help me out, how is that block ever invoked? The above code
> snippet just appends the block as a parameter to an exception, which
> looks just funny.
>
> Niko
>
> On Mon, Oct 4, 2010 at 3:44 PM, Lukas Renggli <[hidden email]> wrote:
>> See for senders of #errorBlock:, e.g.
>> PushDownInstanceVariableRefactoring, RemoveClassRefactoring or
>> RemoveInstanceRefactoring.
>>
>> On 4 October 2010 15:40, Lukas Renggli <[hidden email]> wrote:
>>> This is used to validate the preconditions of refactorings and inform
>>> the user about any mismatches.
>>>
>>> It should better not be removed.
>>>
>>> Lukas
>>>
>>> On 4 October 2010 15:36, Niko Schwarz <[hidden email]> wrote:
>>>> Hi,
>>>>
>>>> The RB refactorings do some obscure dance around an errorBlock, which
>>>> is always nil, as far as I can see. As I was trying to supply a
>>>> non-nil error block, I was astonished to find out that even if it
>>>> isn't nil, it won't be executed. Here's the relevant snippets:
>>>>
>>>> checkPreconditions
>>>>        | conditions block |
>>>>        conditions := self preconditions.
>>>>        conditions check
>>>>                ifFalse:
>>>>                        [block := conditions errorBlock.
>>>>                        block notNil
>>>>                                ifTrue: [self refactoringError: conditions errorString with: block]
>>>>                                ifFalse: [self refactoringError: conditions errorString]]
>>>>
>>>> refactoringError: aString with: aBlock
>>>>        ^ RefactoringError signal: aString with: aBlock
>>>>
>>>> I can't imagine how that could help anyone. I suggest either removing
>>>> the errorBlock, or turning it into a good old instance variable, and
>>>> running it in case of an error.
>>>>
>>>> Cheers,
>>>>
>>>> Niko
>>>>
>>>> --
>>>> http://scg.unibe.ch/staff/Schwarz
>>>> twitter.com/nes1983
>>>> Tel: +41 076 235 8683
>>>>
>>>> _______________________________________________
>>>> Pharo-project mailing list
>>>> [hidden email]
>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>>>
>>>
>>>
>>>
>>> --
>>> Lukas Renggli
>>> www.lukas-renggli.ch
>>>
>>
>>
>>
>> --
>> Lukas Renggli
>> www.lukas-renggli.ch
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>
>
>
> --
> http://scg.unibe.ch/staff/Schwarz
> twitter.com/nes1983
> Tel: +41 076 235 8683
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

> The tool that triggers the refactoring, e.g. ORCommand>>#handleError:
>
> Ok, the code is quite ugly, but this is how the original refactoring
> browser implemented it.
>
> On 4 October 2010 16:06, Niko Schwarz <[hidden email]> wrote:
>> Ok, help me out, how is that block ever invoked? The above code
>> snippet just appends the block as a parameter to an exception, which
>> looks just funny.
>>
>> Niko
>>
>> On Mon, Oct 4, 2010 at 3:44 PM, Lukas Renggli <[hidden email]> wrote:
>>> See for senders of #errorBlock:, e.g.
>>> PushDownInstanceVariableRefactoring, RemoveClassRefactoring or
>>> RemoveInstanceRefactoring.
>>>
>>> On 4 October 2010 15:40, Lukas Renggli <[hidden email]> wrote:
>>>> This is used to validate the preconditions of refactorings and inform
>>>> the user about any mismatches.
>>>>
>>>> It should better not be removed.
>>>>
>>>> Lukas
>>>>
>>>> On 4 October 2010 15:36, Niko Schwarz <[hidden email]> wrote:
>>>>> Hi,
>>>>>
>>>>> The RB refactorings do some obscure dance around an errorBlock, which
>>>>> is always nil, as far as I can see. As I was trying to supply a
>>>>> non-nil error block, I was astonished to find out that even if it
>>>>> isn't nil, it won't be executed. Here's the relevant snippets:
>>>>>
>>>>> checkPreconditions
>>>>>        | conditions block |
>>>>>        conditions := self preconditions.
>>>>>        conditions check
>>>>>                ifFalse:
>>>>>                        [block := conditions errorBlock.
>>>>>                        block notNil
>>>>>                                ifTrue: [self refactoringError: conditions errorString with: block]
>>>>>                                ifFalse: [self refactoringError: conditions errorString]]
>>>>>
>>>>> refactoringError: aString with: aBlock
>>>>>        ^ RefactoringError signal: aString with: aBlock
>>>>>
>>>>> I can't imagine how that could help anyone. I suggest either removing
>>>>> the errorBlock, or turning it into a good old instance variable, and
>>>>> running it in case of an error.
>>>>>
>>>>> Cheers,
>>>>>
>>>>> Niko
>>>>>
>>>>> --
>>>>> http://scg.unibe.ch/staff/Schwarz
>>>>> twitter.com/nes1983
>>>>> Tel: +41 076 235 8683
>>>>>
>>>>> _______________________________________________
>>>>> Pharo-project mailing list
>>>>> [hidden email]
>>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Lukas Renggli
>>>> www.lukas-renggli.ch
>>>>
>>>
>>>
>>>
>>> --
>>> Lukas Renggli
>>> www.lukas-renggli.ch
>>>
>>> _______________________________________________
>>> Pharo-project mailing list
>>> [hidden email]
>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>>
>>
>>
>>
>> --
>> http://scg.unibe.ch/staff/Schwarz
>> twitter.com/nes1983
>> Tel: +41 076 235 8683
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>
>
>
> --
> Lukas Renggli
> www.lukas-renggli.ch
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

>> Ok, let's summarize: If an error occurs, the handler block is not
>> evaluated, but attached to a new exception, that exception is raised.
>> Then, the caller catches that exception, retrieves the handler block,
>> does some more ballyhoo, and then runs the handler block.
>
> Yes, the block mostly contains some code to that allows the user to
> fix the problem (like to show all references of a class to be removed,
> etc.).
>
>> And you get invited to this construction by these methods in
>> RBAbstractCondition:
>>
>> errorBlock
>>        ^self errorBlockFor: false
>>
>> errorBlockFor: aBoolean
>>        ^nil
>
> These methods (and subclass implementations) are used to retrieve the
> right block that caused the composite condition to fail.
>
>> We can clean it up on the next sprint, if you want :)
>
> The implementation in RBCondition is fine, the problem is more the way
> how exceptions are used to pass the values. This mainly stems from the
> fact that exceptions evolved over the past 15 years and are no longer
> the same as the signals that were used when the refactoring code was
> originally written.
>
> Now of course it is difficult to change this code because there are
> various users out there that depend on it, e.g. OB,
> RefactoringBrowser, SmaCC, ...
>
> Lukas
>
> --
> Lukas Renggli
> www.lukas-renggli.ch
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project