Hello leostereo,
Why is it so horrible to read??
Because the haphazard indentation makes it almost impossible to tell, at a glance, where each logical block of code ends. Here’s the original version of sub write_register, with a single comment added:
Now, quickly: which block of code is terminated by the right brace named X?
Here’s a preliminary clean-up of the code, with consistent indentation:
Easier to follow now? Yes, and that makes it easier to spot where it can be improved:
- As noted by Anonymous Monk++, the variables $output and $fver are never declared. In addition, the variable $bsid is neither declared nor initialized anywhere in the script.
- Perl allows you to return, exit, or die at any point in the code. Since these statements are forms of flow control, the code can often be simplified when these statements are used. For example, this:
if (condition)
{
return;
}
else
{
...
}
is logically equivalent to the shorter and simpler:
return if condition;
...
- It’s bad practice to exit from a subroutine. You should die (i.e., throw an exception) instead: that way, the subroutine’s clients have the option to either handle the exception locally or allow it to propagate upwards and so terminate the script.
Here’s an improved version of the subroutine:
As a rule, the shorter and simpler the code, the easier it is to debug and maintain. Disclaimer: I haven’t looked at the code’s logic, only its form.
Hope that helps,
| [reply]
[d/l]
[select] |
