[vlc-devel] [PATCH] Qt/ML: New Models for Playlist UI

[Jean-Baptiste Kempf]

On Tue, Dec 28, 2010 at 02:55:45AM +0530, Srikanth Raju wrote :
> Right patch attached. :|

As much as I don't like the one-big-patch way especially because a
first patch to move the PLModel to VLCModel+PLModel was necessary, I
won't nitpick too much now.

 - I don't see why you need typeToString(  ) since you only use it once to
   store in a QVariant::Data

 - getStringFromPeople could be simplified by using QStrings... Not to
   mention that you use it only once and convert it to QString
   afterwards...

 - case ML_DURATION: is overly ugly, when you could do a
   QTime().addSecs( media->i_duration/1000000 ).toString( HH:mm:ss);

 - case ML_TITLE could be done better with QFileInfo

 - strdup( qtu(data.toString()) ) is dubious, in setData

 - In int MLModel::insertMediaArray, declare i in the for loop, unless
   necessary. Same for MLModel::insertResultArray

 - Shouldn't MLModel::appendResult be inlined?

 - Shouldn't MLModel::popup be mutualized between models?

That's all I could see in the first pass


Best Regards,

-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/
+33 672 704 734