search

fruitl00p / php-mt940

A mt940 parser in PHP
MIT License
103 stars 60 forks source link

Transaction::getAccountName return type #26

Closed SamMousa closed 7 years ago

SamMousa commented 8 years ago

Transaction::getAccountName() should return null when no name is present in the transaction; returning an empty string is not the same.

fruitl00p commented 8 years ago

I feel that changing the return type would be worse than an empty string...

SamMousa commented 8 years ago

The point is that null in indicates a missing value while an empty string is a valid name.

On Thu, Dec 24, 2015, 19:11 Robin Speekenbrink notifications@github.com wrote:

I feel that changing the return type would be worse than an empty string...

— Reply to this email directly or view it on GitHub https://github.com/fruitl00p/php-mt940/issues/26#issuecomment-167144180.

fruitl00p commented 8 years ago

I know blank is an invalid name, and as a setter it would be invalid, but the official mt940 docs (AFAIK) dont mention that a name has a minimum of characters, thus not switching return type has my preference... One might argue about throwing an exception, but thats a different story ;)

SamMousa commented 8 years ago

If mt940 doc don't have a minimum length then blank is valid.. That is why we need null. Note that null is not a different type; it is not a type at all

On Sun, Dec 27, 2015, 19:39 Robin Speekenbrink notifications@github.com wrote:

I know blank is an invalid name, and as a setter it would be invalid, but the official mt940 docs (AFAIK) dont mention that a name has a minimum of characters, thus not switching return type has my preference... One might argue about throwing an exception, but thats a different story ;)

— Reply to this email directly or view it on GitHub https://github.com/fruitl00p/php-mt940/issues/26#issuecomment-167433308.

fruitl00p commented 8 years ago

null is a special type indeed, thats true... Anyone else have an idea of switching return types ?

fruitl00p commented 7 years ago

I still stand by my original point of view that returning NULL in this situation is weird? The return type is a string (it might be empty) but it is / should always be a string...