On Saturday 21 January 2012, 01:39, Matěj Laitl wrote:
> Git commit 99a7d0b5bb541ed6400213d81cc7683935666514 by Matěj Laitl.
> Committed on 20/01/2012 at 15:21.
> Pushed by laitl into branch 'master'.
>
> CollectionLocation: make associated collection pointer non-const
>
> CollectionLocation is used to copy/move/delete tracks from collection,
> it is irrational that it would point to _constant_ parentCollection.
>
> Also make Collection::location() non const, same rationale. This allows
> to get rid of a fair bunch (well, 7) of const_casts.
>
> _All_ places where there methods/arguments/attributes are updated to
> reflect the change (even ones that would compile without changing).
>
> Furthermore {DatabaseCollection,SqlCollection}::is{Writable,Organizable}
> can use the default implementation, which is tweaked to be null pointer
> safe.
>
> Many classes that subclass CollectionLocation called its default
> constructor, this is now fixed to call its
> CollectionLocation( Collection *parentCollection ) constructor so that
> default implementation of CollectionLocation::collection() works.
>
> (UmsCollectionLocation even introduced its own ::umsCollection() method
> probably because generic collection() did not work (surprise), it is
> removed in favor of default ::collection() implementation)
>
> (snip)
Hey there,
Disclaimer: I don't really know the code, so forgive me when I'm wrong.
I'm curious. Why does Collection::location() need to be non-const? From what I
can see in your diff you can make it const, after all. (=> no const_cast
needed at all)
Secondly, it should be named createLocation() since it is in fact allocating
memory on the heap on every call, isn't it? That applies both to
Collection::queryMaker and Collection::location, actually. They should be
named createQueryMaker and createLocation respectively and made const to
reflect their behavior (I refer to the factory pattern here).
(SqlCollection, for example, internally calls "QueryFactor::createQueryMaker()
const" which is what I would expect).
Just my two cents. :)
Greets
--
Kevin Funk
_______________________________________________
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel
