----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128624/#review98235 -----------------------------------------------------------
kmymoney/plugins/csvimport/csvdialog.h (line 64) <https://git.reviewboard.kde.org/r/128624/#comment66174> Either use non_camel_case or CamelCase but please not both at the same time. As Cristian suggested, use typedef enum { ... } ColumnType_t and then use ColumnType_t as the type whereever you need it. This way you get type checking for free. Use static_cast<ColumnType_t>(xxx) to convert an e.g. an int xxx. kmymoney/plugins/csvimport/csvdialog.cpp (line 303) <https://git.reviewboard.kde.org/r/128624/#comment66168> The QString ctor does that for you. Using this kind of initialization is pure overhead. kmymoney/plugins/csvimport/csvdialog.cpp (line 430) <https://git.reviewboard.kde.org/r/128624/#comment66170> why toUtf8() here? A QString is per se UTF. kmymoney/plugins/csvimport/csvdialog.cpp (line 434) <https://git.reviewboard.kde.org/r/128624/#comment66171> Isn't QSet<QString> a better solution for m_hashMap than QMap<QString,bool> here? kmymoney/plugins/csvimport/csvdialog.cpp (line 811) <https://git.reviewboard.kde.org/r/128624/#comment66175> You should have an InvalidColumn or similar. uchar() might end up with 0 which is a valid column type (ColumnNumber) kmymoney/plugins/csvimport/csvwizard.cpp (line 369) <https://git.reviewboard.kde.org/r/128624/#comment66173> m_csvDialog->Column_Credit might work here. Please use CsvDialog::Column_Credit when you use a constant (enum) defined as part of a class. This applies to all other location of this file as well. - Thomas Baumgart On Aug. 7, 2016, 5:40 nachm., Łukasz Wojniłowicz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128624/ > ----------------------------------------------------------- > > (Updated Aug. 7, 2016, 5:40 nachm.) > > > Review request for KMymoney. > > > Repository: kmymoney > > > Description > ------- > > 1) processQIFLine should identify fields by integers and not strings (better > performance), > 2) validation of debit and credit column contained unnecessary check and was > complicated, > 3) QIF creation is not essential to processing, > 4) statements is not needed and consumed memory exponentially, > 5) cleaner hash assignation, > 6) lots of redundant variables. > > createMemoField is commented for now but won't be after I rewrite > processInvestLine. > > > Diffs > ----- > > kmymoney/plugins/csvimport/csvdialog.h 65bbeb7 > kmymoney/plugins/csvimport/csvdialog.cpp 6d91d63 > kmymoney/plugins/csvimport/csvwizard.h 2743685 > kmymoney/plugins/csvimport/csvwizard.cpp b042a98 > > Diff: https://git.reviewboard.kde.org/r/128624/diff/ > > > Testing > ------- > > > Thanks, > > Łukasz Wojniłowicz > >
