D14523: [import] Update file content overview after file type change

[Alexander Semke]

asemke added inline comments.

INLINE COMMENTS

> fkristof wrote in ImportFileWidget.cpp:612
> I'd say it would be safer to have 2 separate ifs, even if we know that the order is left to right, if somebody changes/adds here something and the fileName.at(0) will be before the isEmpty() then there will be a crash.
> 
>   if (!fileName.isEmpty()) {
>          if (fileName.at(0) != QDir::separator()) {

This part Christoph copied simply from another place... This logic we have in many places in the code where we striclty rely on the fixed ordering of the evaluation which has also to be guaranteed by the compiler. Many people are used to this and I think this is ok...

> croick wrote in ImportFileWidget.cpp:624
> You meant `const QString fileName`, right?

I meant const reference. Though the difference with respect to the performance between copying an object and creating a reference to it might be very small for implicitly shared Qt classes (QString is implicitely shared), it's still matter of convention and of code styly. With const QString& you _explicitly_ say I do not want to copy and I'm not going to change the reference - this is what we try to consistenly use throughout the code.

> croick wrote in ImportFileWidget.cpp:843
> Cannot make it const, it's modified later on.

ok.

> croick wrote in ImportFileWidget.h:111
> No, but there were only slots so far (also unconnected).

Ok. We should clean up here a bit maybe later and make private functions. Thanks for the pointer.

REPOSITORY
  R262 LabPlot

REVISION DETAIL
  https://phabricator.kde.org/D14523

To: croick, #labplot
Cc: fkristof, asemke, kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20180801/969353bc/attachment.html>