http://qs1969.pair.com?node_id=1179978


in reply to Re: Improve readability of Perl code. Naming reference variables.
in thread Improve readability of Perl code. Naming reference variables.

Yeah. I completely agree with you with the idea of keeping subroutines or code blocks small. It is also my experience that this coding style (that you propose) has potential for eliminating most readability issues. So the naming issue can usually be circumvented using proper naming of variables and keeping the scope small. But it's also my experience that in some cases it would still be beneficial to have the option to further document the type of a reference variable.

Replies are listed 'Best First'.
Re^3: Improve readability of Perl code. Naming reference variables.
by stevieb (Canon) on Jan 20, 2017 at 14:23 UTC

    What you're referring to in your last sentence is what some call "edge cases". These edge cases, where there may be ambiguity to the reader of the code is where your extremely brief comments should go. Code should document itself, but if you feel the reader may scratch their head:

    ... my $x = thing_list(); # href ...

    Of course, that's a pretty trivial example, but you get the point.

      Hi stevieb,

      Yes I think I get the point :) But consider another example, a sub routine is called with three references. A scalar reference, a hash refernece, and an array reference. Here is an example of a corresponding sub definition:

      sub func { my ( $str, $desktop_info, $files ) = @_; #sref, href, aref ... }
      In this case your end-of-line comment could work. But would it not be better if the comment could be avoided? If the variable names were self-documenting there would be no need for the comment. And the code should be easier to maintain. If you change the arguments to func you do not need to also remember to update the comment:
      sub func { my ( $str->$, $desktop_info->%, $files->@ ) = @_; ... }

        Now you've circled back around to scope size again. If your sub is around one screenful of code, it's easy to know what each var is and does, because you can at a glance see it in use without paging or having to search an entire file.

        Then, you have your documentation (you did write documentation, right? ;)

        Often when I'm writing new modules or updating older ones, I have the current documentation open in either a browser or another terminal window. Your documentation will provide you with the function/method declaration, and will clearly describe what each parameter is. I'm not advocating using comments here, I'm just saying in rare cases where it may be ambiguous to the reader, throw in a comment. fwiw, I don't recall over the last year ever having to specify what a variable is via comment in any of my code.

        In your example, with those names, I immediately and instinctively knew which type each param was. If I was to update that sub, the worst thing that would happen would be an error spit out because I mistakenly used the variable as a different type, and since I test code extremely often, there wouldn't be much I'd have to go back and change to correct my oversight.

        This is a nice example of why the way I learned to write subs makes me happy. I'm sharing it with you, so you can join in the fun.

        sub a_subroutine { my ($opt, @bad ) = @_; my $string = delete $opt->{a_descriptively_named_string}; my $desktop_info = delete $opt->{desktop_config}; my $important_files = delete $opt->(important_files} // []; checkopt( \%opt, \@bad, a_descriptively_named_string => defined $string, desktop_config => ref $desktop_info eq 'HASH', important_files => ref $important_files eq 'ARRAY +', ); # Do stuff. } # checkopt is a nice little function along these line. # You can customize it a bit, (add support for testing for invocants, +fancy crap with passing additional error messages, what have you) # But in pretty much this form it has caught made many a bug shallow sub checkopt { while (@_) { if ( ref eq 'HASH' ) { my $opts = shift; @args = keys @$opts; croak "Unexpected options in subroutine call: @args" if %$opts; } elsif ( ref eq 'ARRAY' ) { my $bad = shift; croak "Unexpected arguments in subroutine call." if @$bad } else { my $option_name = shift; my $option_okay = shift; croak "Invalid option '$option_name' in subroutine call" unless $option_okay; } } }

        This code is nice because it is very explicit. It shows you exactly what it expects. It doesn't really rely on any weirdness.

        So we start off by passing in a single hashref with a set of named arguments. Extra arguments are sucked up by the @bad variable.

        We then unpack the options hashref into variables, deleting keys as we go. This ensures that the hash will be empty when all known arguments have been unpacked. This is a handy place to set default values for options.

        Finally we call our check function. We pass in @bad and %opt by reference, to ensure that they are empty. Other arguments are handled by passing in key/value pairs. This version croaks when a false value is found for any key.

        Yes this code is a bit verbose. But it is easy to read, very regular and can contain a lot of information. Including what type each variable/argument is.

        It's also worth thanking tye who wrote the original version of this check function I recalled for you. Any errors in its operation as embodied here are solely mine. The real thing actually works.


        TGI says moo