Review Request: Fix MakeOutput items for cmake builds with relative paths

[Matt Rogers]

On Tue, Mar 1, 2011 at 10:10 AM, Milian Wolff <mail at milianw.de> wrote:
> Andreas Pakulat, 01.03.2011:
>> On 01.03.11 13:39:26, Milian Wolff wrote:
>> > Manuel Massing, 01.03.2011:
>> > > Hi Milian,
>> > >
>> > > > yes, I'd prefer a single patch for that as it belongs together imo.
>> > >
>> > > ok, will do when I'm at home.
>> > >
>> > > > Furthermore the wrong i18n usage still persists in the patches you
>> > > > send. Please fix that.
>> > >
>> > > The i18n usage should be fixed in the patches I sent to you. It may
>> > > have looked like I addressed your comments only after sending the
>> > > patch, because I submited but forgot to publish the comments regarding
>> > > your review.
>> > >
>> > > > Also - correct me if I'm wrong - I fear your code is overly complex:
>> > > > Imo the map with the iterators is not needed, the index lookup in a
>> > > > QList is
>> > >
>> > > > O(1) and hence you should use that for currentDirs. See also:
>> > > to avoid O(n^2) we want sub-linear key lookup - which is O(log N) with
>> > > the map but O(n) with QList (i.e. despite the naming, indexOf is a
>> > > key-lookup!)
>> >
>> > a QList has no keys, hence it cannot be a key-lookup. And the Qt
>> > documentation is clear on this: Index lookup is O(1).
>>
>> You two are talking about different things. indexOf() in QList is definetly
>> not O(1), what is O(1) are at() and [] indexing into a qlist. Thats two
>> different things. And in fact that table does not make any kind of
>> guarantees about the complexity of finding the index of an element in a
>> QList.
>
> Doh, you are right. I've looked at the implementation of QList::indexOf, it
> should bit O(n).
>
> So if this map really is such a good idea and brings a noticeable performance
> gain I'm OK with the patch.
>
> bye


If it's really such a good idea, why isn't the data structure a map to
begin with? >:)
--
Matt