Empty string reported via constructor

samseaver commented 12 years ago

So, this really threw me for a while, and this is because the error message was actually wrong. I realized that I had a reaction for which there was no name available, but when running addrxntable, the error message I got was this:

Attribute (name) does not pass the type constraint because: The string you provided () is too long to be a varchar! at constructor ModelSEED::MS::Reaction::new (defined at /Users/seaver/Software/ModelSEED/lib/ModelSEED/MS/Reaction.pm line 371) line 176
        ModelSEED::MS::Reaction::new('ModelSEED::MS::Reaction', 'HASH(0x55aa2d0)') called at /Users/seaver/Software/ModelSEED/lib/ModelSEED/MS/Biochemistry.pm line 469
        ModelSEED::MS::Biochemistry::addReactionFromHash('ModelSEED::MS::Biochemistry=HASH(0x44ace30)', 'HASH(0x55a7da0)') called at /Users/seaver/Software/ModelSEED/lib/ModelSEED/App/bio/Command/addrxntable.pm line 50
        ModelSEED::App::bio::Command::addrxntable::execute('ModelSEED::App::bio::Command::addrxntable=HASH(0xc3ef60)', 'Getopt::Long::Descriptive::Opts::__OPT__::4=HASH(0x906280)', 'ARRAY(0x905c20)') called at /Library/Perl/5.10.0/App/Cmd.pm line 243
        App::Cmd::execute_command('ModelSEED::App::bio=HASH(0xc0e6a0)', 'ModelSEED::App::bio::Command::addrxntable=HASH(0xc3ef60)', 'Getopt::Long::Descriptive::Opts::__OPT__::4=HASH(0x906280)', 'KEGG', '/Users/seaver/Documents/Projects/Model_Import_Files/KEGGimpor...') called at /Library/Perl/5.10.0/App/Cmd.pm line 171
        App::Cmd::run('ModelSEED::App::bio=HASH(0xc0e6a0)') called at /Users/seaver/Software/ModelSEED/lib/ModelSEED/App/mseed/Command/bio.pm line 18
        ModelSEED::App::mseed::Command::bio::execute('ModelSEED::App::mseed::Command::bio=HASH(0xc2ab20)', 'Getopt::Long::Descriptive::Opts::__OPT__::2=HASH(0xc0efa0)', 'ARRAY(0x89a6d0)') called at /Library/Perl/5.10.0/App/Cmd.pm line 243
        App::Cmd::execute_command('ModelSEED::App::mseed=HASH(0x8b1f40)', 'ModelSEED::App::mseed::Command::bio=HASH(0xc2ab20)', 'Getopt::Long::Descriptive::Opts::__OPT__::2=HASH(0xc0efa0)', 'addrxntable', 'KEGG', '/Users/seaver/Documents/Projects/Model_Import_Files/KEGGimpor...') called at /Library/Perl/5.10.0/App/Cmd.pm line 171
        App::Cmd::run('ModelSEED::App::mseed') called at /Users/seaver/Software/ModelSEED/bin/ms line 8
        main::__ANON__() called at /Library/Perl/5.10.0/Try/Tiny.pm line 76
        eval {...} called at /Library/Perl/5.10.0/Try/Tiny.pm line 67
        Try::Tiny::try('CODE(0x813c00)', 'Try::Tiny::Catch=REF(0x8819d0)') called at /Users/seaver/Software/ModelSEED/bin/ms line 16

so I was looking for a name that was too long. In any case, I think the simple case of an empty string being passed via a new() function needs to be handled properly.

devoid commented 12 years ago

I'll admit that the error message could be better constructed. My tests indicate that you're passing in undef to the constructor, which will cause it to die. If you pass in an empty string it will simply return an empty string. If you pass in no value, it will return an empty string:

sub cpd { return ModelSEED::MS::Compound->new($_[0]) }

print cpd({ name => "" })->name; # prints ""
print cpd({})->name; # prints ""
print cpd({ name => undef })->name; # dies

I'm not sure what the correct behavior is if you pass in undef. Frankly the empty strings seems like the wrong way as well...more likely to confuse people than an undefined value. But I could go either way on this.

samseaver commented 12 years ago

This indicates that the hash which stores the properties should have some defaults.

ModelSEED / ModelSEED

Empty string reported via constructor #111