Review Request 128624: Rewrite processQIFLine

[Thomas Baumgart]

-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20160809/6808e664/attachment-0001.html>