Review Request: Helper Methods for FileType Enum in new Class FileTypeSupport, get Type in TagDialog from Database instead of Extension, some Code Changes as Example of the Advantage of the Helper Methods

[Sergey Ivanov]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100088/#review301
-----------------------------------------------------------



shared/FileType.cpp
<http://git.reviewboard.kde.org/r/100088/#comment246>

    What's a point to have "Other" as supported/possible file types? Tbh, I don't know where It could be used. I can be wrong.
    But If remove this, you were not need KLocale, and you can change ( a bit ) enum with types, to make It like Unknown = -1, Mp3 = 0, ..., so you can just return Amarok::FileType( list.indexOf( extension.toLower() ) );



shared/FileType.cpp
<http://git.reviewboard.kde.org/r/100088/#comment245>

    It can be started from 1, since no one needs "Other" extension. :)


And what relates to entire patch. As Leo noticed, you should use amarok code formating style. And It would be better to set const modifier to func args since you don't going to modify 'em, and use links instead of copying  whole variable in case of strings.

- Sergey


On 2010-11-05 10:46:54, Stefan Derkits wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100088/
> -----------------------------------------------------------
> 
> (Updated 2010-11-05 10:46:54)
> 
> 
> Review request for Amarok.
> 
> 
> Summary
> -------
> 
> -) Created new Class with some Helper Functions
> -) Changed MetaQueryWidget to use the QStringList that contains all Filetypes
> -) Meta::SqlTrack.type() now gets it's Information from the Database instead of the FileExtension
> 
> 
> Diffs
> -----
> 
>   src/browsers/CollectionTreeItemModelBase.cpp 3f64a73 
>   shared/FileType.cpp PRE-CREATION 
>   shared/FileType.h 55c80b9 
>   src/core-impl/collections/support/MemoryFilter.cpp e2509ac 
>   src/widgets/MetaQueryWidget.cpp 0249edf 
>   tests/synchronization/CMakeLists.txt e5df2df 
> 
> Diff: http://git.reviewboard.kde.org/r/100088/diff
> 
> 
> Testing
> -------
> 
> Tested in the App, found no wrong Behaviour
> 
> 
> Thanks,
> 
> Stefan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/amarok-devel/attachments/20101105/86c13b55/attachment.htm