ochurlaud / macaw-movies

Movie collection manager, for movie lovers. Qt/C++/Sqlite |
GNU General Public License v3.0
26 stars 4 forks source link

Adding media player selection to the settings menu (part of Menus #54) #123

Closed jrutanen closed 9 years ago

jrutanen commented 9 years ago

Menus #54

Wrong SQLite data type in config table #122

ochurlaud commented 9 years ago

Ok everything worked well this time. I see you changed the git name, which is perfect. I still want to check the code, but as far as I've seen it's ok.

2 improvements still:

ochurlaud commented 9 years ago

in DatabaseManager::addMediaPlayerPath(QString mediaPlayerPath)

Why don't you directly remove and, if the string is not empty, insert? Here you've got a lot of logic (get the value, compare, update or insert depending of the state...).

ochurlaud commented 9 years ago

It's alright

jrutanen commented 9 years ago

You're right the addition is too complicated. Remove+insert is much more straight forward. I'll change that.

jrutanen commented 9 years ago

Thanks for a lot for the review.