Phase 2 commits

Thomas Baumgart thb at net-bembel.de
Fri Aug 14 09:07:13 BST 2020


Hi Prasun,

On Samstag, 8. August 2020 05:28:37 CEST Prasun Kumar wrote:

> Hi mentors,
> 
> The rest of the work from the second phase has been completed. The databases
> for the additional countries have been incorporated in the library. Please
> take a look
> and suggest any necessary changes.

Sorry for being a bit late, but I was swamped with work during recent weeks.

I took a look at the changes you made after July 24th. They look good in general.
I have a few comments, which I will relate to the commits, though:

396718222dd063f52bb2c7de461fd6bd03e45b45:
2c2e6a5e8856997c94cf80969be503d43fc0ed52:

Why did you rename Switzerland to Swiss? I don't see a necessity to do so in
the first place. After all, it is still Netherlands and not Dutch. The country's
name is Switzerland and they are swiss people. It's like India and Indian.



0bdd7fb302190697d2fc7170f0f455c34b639b1d:

        if (accountId.size() > 12 || bankId.size() != 5) {
            return ERROR;
        }

Where does 12 and 5 come from? Please use symbolic constants as in

#define NL_MIN_ACCOUNT_ID_SIZE (12) and
#define NL_BANK_ID_SIZE        (5)

Note: I guessed the names and have no idea if they are correct. They
should simply serve as example.


0d473847b3966c4b321cbf52360ea4451d3eeab1:

        char *bankId = new char[6];
        char *name = new char[61];

Same here: please use symbolic constants so that one knows where the size comes from.
If you need an extra byte for the trailing zero it is a good practice to do it like
this (example):

#define NL_BANK_ID_SIZE        (5)

        char *bankId = new char[NL_BANK_ID_SIZE + 1];

        if (bankId.size() != NL_BANK_ID_SIZE) {



So much from my end. Keep going.

Regards

Thomas



-- 

Regards

Thomas Baumgart

https://www.signal.org/       Signal, the better WhatsApp
-------------------------------------------------------------
As soon as we wish to be happier, we are no longer happy. -- Walter Landor
-------------------------------------------------------------
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 868 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-finance-apps/attachments/20200814/7d7bc9fc/attachment.sig>


More information about the Kde-finance-apps mailing list