> On Sept. 7, 2016, 8:07 vorm., Thomas Baumgart wrote: > > kmymoney/plugins/csvimport/csvwizard.cpp, line 625 > > <https://git.reviewboard.kde.org/r/128817/diff/2/?file=475985#file475985line625> > > > > Doesn't that match an empty txt as well? > > Łukasz Wojniłowicz wrote: > Yes it does and couple of lines above is my recipe for it > ```c++ > QString txt = ui->tableWidget->item(row, col)->text(); > if (txt.isEmpty()) // nothing to process, so go to next row > continue; > ```
Never mind. I did not see that. Then it's not a problem. > On Sept. 7, 2016, 8:07 vorm., Thomas Baumgart wrote: > > kmymoney/plugins/csvimport/csvwizard.cpp, line 622 > > <https://git.reviewboard.kde.org/r/128817/diff/2/?file=475985#file475985line622> > > > > Don't we know about a lot more currency symbols in the engine > > (MyMoneyFile::instance()->currencyList())? The method would be > > tradingSymbol(). See KMyMoneyView::loadDefaultCurrencies() for the list of > > known currencies. Why do some of them are hard coded here? > > Łukasz Wojniłowicz wrote: > I use d following code to harvest potential currencies. > ```c++ > QList<MyMoneyAccount> accountList; > MyMoneyFile::instance()->accountList(accountList); > > QList<MyMoneyAccount::accountTypeE> accountTypes; > accountTypes << MyMoneyAccount::Checkings << > MyMoneyAccount::Savings << > MyMoneyAccount::Liability << > MyMoneyAccount::Checkings << > MyMoneyAccount::Savings << > MyMoneyAccount::Cash << > MyMoneyAccount::CreditCard << > MyMoneyAccount::Loan << > MyMoneyAccount::Asset << > MyMoneyAccount::Liability; > > QString filteredCurrencies; > QList<MyMoneyAccount>::ConstIterator account; > for (account = accountList.cbegin(); account != accountList.cend(); > ++account) > if (accountTypes.contains((*account).accountType()) && // account > must acctually have currency property > !filteredCurrencies.contains((*account).currencyId())) // > currency must not be on the list already > filteredCurrencies += (*account).currencyId(); > ``` > > Łukasz Wojniłowicz wrote: > Disregard previous response. It should be this. > ```c++ > QList<MyMoneyAccount> accountList; > MyMoneyFile::instance()->accountList(accountList); > > QList<MyMoneyAccount::accountTypeE> accountTypes; > accountTypes << MyMoneyAccount::Checkings << > MyMoneyAccount::Savings << > MyMoneyAccount::Liability << > MyMoneyAccount::Checkings << > MyMoneyAccount::Savings << > MyMoneyAccount::Cash << > MyMoneyAccount::CreditCard << > MyMoneyAccount::Loan << > MyMoneyAccount::Asset << > MyMoneyAccount::Liability; > > QString filteredCurrencies; > QList<MyMoneyAccount>::ConstIterator account; > for (account = accountList.cbegin(); account != accountList.cend(); > ++account) { > if (accountTypes.contains((*account).accountType()) && // account > must actually have currency property > !filteredCurrencies.contains((*account).currencyId(), > Qt::CaseInsensitive)) { // currency id must not be on the list already > filteredCurrencies += (*account).currencyId(); > QString tradingSymbol = > MyMoneyFile::instance()->currency((*account).currencyId()).tradingSymbol(); > if (!filteredCurrencies.contains(tradingSymbol, > Qt::CaseInsensitive )) // currency trading symbol must not be on the list > already > filteredCurrencies += tradingSymbol; > } > } > ``` > I think it's better to remove currency symbols based on what user has in > his accounts, than to hunt for over 180 currency symbols/ids from > (MyMoneyFile::instance()->currencyList()). I'd use a QSet<QString> to add all trading symbols w/o the hassle of checking if it already exists. QSet does not create duplicates. At the end QSet<QString> symbols; // do the collection of the symbols over the accounts here QString filteredCurrencies = QStringList(symbols.values()).join(""); Should make the code more readable. - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128817/#review98964 ----------------------------------------------------------- On Sept. 7, 2016, 8:31 nachm., Łukasz Wojniłowicz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128817/ > ----------------------------------------------------------- > > (Updated Sept. 7, 2016, 8:31 nachm.) > > > Review request for KMymoney. > > > Repository: kmymoney > > > Description > ------- > > I actually decided to get rid of completion page and restructure LinesDate > page. In my opinion it's more logical now and better structured for new > feature I introduced ie. calculate fee column on fly. > > 1) shortened updateDecimalSymbol and renamed it to > validateDecimalSymbol, > 2) introduced detectDecimalSymbol for autodetecting decimal symbol, > 3) simplified createStatement, > 4) simplified and deduplicated slotImportClicked, > 5) new decimal symbol option 'auto' for columns with various decimal > symbols, > 6) decimal symbol and date format get checked every time formats page is > being opened, > 7) removed completion page, > 8) introduced rows page after separator page which is usefull for > calculate fee > 9) introduced formats page which groups all validity checks in one > place, > 10) checking securities existence is now part of > InvestmentPage::validatePage(), > 11) validateDateFormat operates on tableWidget and not on m_lines so it > can be faster, > 12) renamed page labels so they represent the content better, > 13) Rows and Formats page have layout equal to Separator page, so look is > more consistent. > > > Diffs > ----- > > kmymoney/plugins/csvimport/CMakeLists.txt 216c79c > kmymoney/plugins/csvimport/completionwizardpage.ui 616e0c3 > kmymoney/plugins/csvimport/csvdialog.h 59e6898 > kmymoney/plugins/csvimport/csvdialog.cpp ed9e1d8 > kmymoney/plugins/csvimport/csvwizard.h ecec5b0 > kmymoney/plugins/csvimport/csvwizard.cpp b576dea > kmymoney/plugins/csvimport/csvwizard.ui 723e128 > kmymoney/plugins/csvimport/formatswizardpage.ui PRE-CREATION > kmymoney/plugins/csvimport/investprocessing.h 902f9bb > kmymoney/plugins/csvimport/investprocessing.cpp f050441 > kmymoney/plugins/csvimport/lines-datewizardpage.ui 84c1c91 > kmymoney/plugins/csvimport/rowswizardpage.ui PRE-CREATION > kmymoney/plugins/csvimport/separatorwizardpage.ui 2ad5abd > > Diff: https://git.reviewboard.kde.org/r/128817/diff/ > > > Testing > ------- > > > Thanks, > > Łukasz Wojniłowicz > >
