On 17-4-2011 17:30, venom00 wrote:
>> We try to sort the includes.
>
> OK
>
>> #if QT_VERSION >= 0x040700
>>> + search_->setPlaceholderText(tr("Search"));
>> #endif
>
> Nice.
>
>> if (!item->child(i)->isDisabled()) {
>> (although now I see this is introduced in Qt 4.3)
>
> I was using other flags before the final patch, isDisabled is better.
I don't know whether we can use it as it was only introduces in Qt 4.3. Then I
started wondering why such a basic feature was introduced this late.
We support Qt versions from 4.2.2.
>
>> It's strange that Qt doesn't paint the items in the Disabled
>> color whenever the item is disabled.
>
> I agree.
>
>> "this" is unnecessary.
>
> More clear IMHO. However I'll remove it.
In principle, wherever you see a variable ending on '_'. This already indicates
that the variable is a private class memeber.
>
>> Usually we use iterators like:
>>
>> PanelMap::const_iterator it = panel_map_.begin();
>> PanelMap::const_iterator end = panel_map_.end();
>> for (; it != end; ++it) {
>> QTreeWidgetItem * item = *it;
>> [..]
>> }
>
> I think this syntax it's very hard to understand, we should use Qt's foreach
> macro (if we'll ever want to drop Qt it's just a file to import), much more
> elegant. However if it's a strict coding rule I'll follow it.
We use it everywhere, so if you are used to it it isn't hard to understand.
You can also use some foreach macro. I just didn't like the use of a
QHashIterator.
>
> About the "if else if else if..." question:
>
>>>> This is really ugly, but I can't think of how to solve it right now.
>
> I know, see my comment. I asked on #Qt [1] and this seems to be the best
> solution. If we want something more clean and OO we should subclass each
> widget
> we want to be searchable, but it's not a good solution IMHO. It would be
> interesting to have somthing in QWidget like QWidget::ToString() or even
> QObject::ToString(), like in .Net.
>
Maybe we should make the individual panels of a type Panel, and then add a
function addWidget(QWidget * widget, QString search_string). So, we just supply
the string everytime we add a widget to a panel. Then we don't need this large
if..else part, we don't need the casts and maybe we can use a
smart way to use them for searching and speed up the search. Iterating over all
widgets seems a bit too much work.
Vincent
