I love this mailing list!
Once again, Uri, thank for your fruitful suggestions. It took me an afternoon
of googling to understand them, but I see now that your suggestions are indeed
“cleaner code” and I will implement them from now on.
Mostly I want to thank you for nailing me on using globals to pass data. I’ve
come across this suggestion in the past but failed to appreciate it. Now I see
that I have a lot of rewriting to do. An uphill job, but it needs to happen!
Rick
> On Sep 9, 2018, at 1:30 PM, Uri Guttman <u...@stemsystems.com> wrote:
>
> On 09/09/2018 02:11 PM, Rick T wrote:
>> I don’t know whether my difficulty is with my perl or with my logic; but
>> either way I need a fresh mind on this.
>>
>> If I test the subroutine by feeding it a file name that does not exist in
>> any format, I expect it to die — but it does not.
>>
>> Any good ideas will be greatly appreciated!
>>
>> Rick
>>
>>
>> sub get_course_data {
>> # check for course_file in .txt format; try to make one if not
>> if ( (!-e "$pf{course_file}.txt" ) # if there's no .txt file
>
> that is doing a hash lookup on the %pf hash. is that what you want?
>
>> and
>> ( -e "$pf{course_file}.db" ) # but there is a .db file
>> ){
>> convert_course_file();
> how does that sub know what file to process?
>
>> }
>> else {
>> die "Cannot find course file for $pf{course_file}";
>> }
>>
>
> it would be much cleaner to rewrite that sub like this (untested rough code):
>
> sub get_course_data {
>
> # don't use globals to pass in data like your code does
>
> my( $course_base ) = @_
>
> my $txt_file = "$course_base.txt" ;
>
> # this could be a one liner with a if modifier.
> # convert_course_file may not return anything useful but we leave anyway as
> we found the file
>
> if ( -e $txt_file ) {
> return convert_course_file( $txt_file ) ;
> }
>
> my $db_file = "$course_base.db" ;
>
> if ( -e $db_file ) {
> return convert_course_file( $db_file ) ;
> }
> die "can't find any $course_base file" ;
>
>
> alternative is to loop over the suffixes as most of the code is duplicated:
>
> foreach my $suf ( qw( txt db ) ) {
>
> my $file_name = "$course_base.$suf" ;
> if ( -e $file_name ) {
> return convert_course_file( $file_name ) ;
> }
> }
>
> die "painful death because $course_base ain't there" ;
>
>
