[Kde-bindings] name conflict between Qt::Widget#raise and Kernel#raise

[Richard Dale]

On Monday 25 May 2009 07:39:59 am Stefano Crocco wrote:
> On Sunday 24 May 2009, Gary Greene wrote:
> > |On Sunday 24 May 2009 09:45:27 am David Palacio wrote:
> > |> On Domingo 24 Mayo 2009 07:35:52 Stefano Crocco escribió:
> > |> > From inside a Qt::Widget, you can't raise an exception using raise,
> > |> > because it's shadowed by Qt::Widget#raise method.
> > |> > What I'd expect, of course, is a RuntimeError raised by the last
> > |> > line of code. Instead, ruby attempts to call Qt::Widget#raise and
> > |> > gives a NoMethodError because Qt::Widget#raise takes no argument.
> > |> >
> > |> > I can solve this problem by calling Kernel.raise instead of simply
> > |> > raise, but I think this is:
> > |> > a) confusing for the user, who might even not be aware that raise is
> > |> > a method and think it a keyword
> > |> > b) undesirable, because I think it's much more common to want to
> > |> > raise an exception than to use Widget#raise.
> > |> >
> > |> > In my opinion, the best course of action would be to rename
> > |> > Qt::Widget#raise to Qt::Widget#raise_widget or
> > |> > Qt::Widget#raise_to_top or something similar, so that Kernel#raise
> > |> > can be freely accessed.
> > |> >
> > |> > Stefano
> > |>
> > |> That is expected in Ruby's inheritance.
>
> I know this, but overriding the base class's version of a method is usually
> done when the new method is meant to replace the functionality of the
> original one, not when it does something completely unrelated.
>
> > |> And «raise» is not the only
> > |> method overriden in a Qt class (as «exec», «load» and others).
>
> I'm aware of this, but in other cases this happens in class less likely to
> be subclassed (this is the case for load) or for less-used methods (as in
> the case of exec).
>
> > |> Changing
> > |> this will only make QtRuby have many special cases (and break existing
> > |> code). IMO, Kernel.method is the best solution for these rare cases.
>
> I admit I didn't think about breaking existing code.
>
> > |> And actually, I wouldn't raise an Exception inside a Widget. Instead,
> > |> I'd emit a signal.
> > |
> > |I agree with David here.
> > |
> > |If you're working on the model, instead of the view, I could see raising
> > | an exception, however when you're working directly on widgets, you're
> > | in the view. The UI should never be treated as part of the model, as it
> > | is a clear violation of the model/view principle.
> > |
> > |Classes that raise exceptions should not inherit from Qt::Widget, rather
> > | they should be either custom worker classes derived from the Ruby
> > | hierarchy of classes, or inherit from Qt::Object.
>
> In my opinion, there might be reasons to raise exceptions from widgets,
> too. For example, I found this out in the following situation: I have a
> KDE::MultiTabBar where the application and its plugins can add tabs, but I
> want to avoid having two tabs with the same caption. What should I do if
> this happens? I think the correct answer is: raise an exception. I admit
> situations you need to do so are rare, but all the same, I'd prefer not to
> have such a core feature of the language be hidden by a method.
>
> There can be another solution to this issue, which didn't occur to me
> yesterday: since Qt::Widget#raise doesn't take any argument, while
> Kernel#rise does, couldn't it be possible to decide which method to call
> depending on the number of arguments? (of course, this would fail in the
> case when the user wants to call Kernel#raise without arguments to re-raise
> the current exception, but it's still better than nothing).
>
> At any rate, if you think that the drowbacks of changing the current
> behaviour exceeds the benefits, at least, please, add a note about this
> issue in the documentation, so that an user which finds this problem can
> find an explaination quickly (as I already noted, there might be people
> thinking that raise is a keyword and thus not expecting it to be
> overridden. I speak from personal experience. The same issue already
> happened to me with the ruby bindings of another GUI toolkit).
Yes, I agree we could do with more documentation about this sort of thing. It 
isn't obvious which is the best thing to do here, and your idea of looking for 
raise() calls with zero args looks the least intrusive. Maybe I prefer 
renaming it to 'raiseWidget()'. 

Otherwise we would need to put special case ruby code in every single class 
that was a subclass of Qt::Widget which seems a bit clunky, although it would 
be less difficult to do it as a C method rather than a Ruby one.

-- Richard