1) should a subroutine do one thing and one thing well rather than trying to combine both subroutines into one because of the common code between them. In other words I could use one subroutine to use the common code but then would have to make it more complex and test whether I was asking for a ticket or a just requesting data from the controller.That's something of a philosophical question... In general, "do one thing, well" is a good practice, but adding unnecessary complexity is also best avoided. In this case, from the brief description you've provided, I'd probably go with two separate functions, get_ticket and request_data (or whatever) and avoid code duplication by having get_ticket call request_data to get the data it needs from the controller.
2) the return value from the ticket and data subroutines are values, scalar for the ticket, and a reference to a data structure for the data subroutine, the exact type of structure depending on what JSON sent back. So if the ticket request or data request fails then the return value is undef. Each time i call the subroutine and get a return value i then have to do an if loop to check whether the value is defined or not. Is there a more efficient way of doing this because I am basically just repeating if statements after each call eg. if i didn't get a ticket I could just call exit in the subroutine but it seems more logical to me and easier to follow if I handle it outside the subroutine.Well, you obviously need to check for errors with each operation. It's just a question of how.
Explicitly checking for errors after each call by testing the definedness (or simply the truthiness) of $ticket is one way to do that. The other common way is to use an exception-based model, in which the sub calls die if something goes wrong and then the calling code can either catch the exception (I recommend Try::Tiny for this) if it's something recoverable, or ignore it and let the exception terminate the program if not.
Either way, though, you should never, ever, ever exit in response to an error in a subroutine, because exit terminates the program immediately without giving the calling code (or any code further up the call stack) an opportunity to examine the problem and attempt to recover. Even if it's an unrecoverable error today, that might change six months from now, or the code could get reused in a different context where recovery may be possible.
Also, die allows you to provide an error message indicating what went wrong; exit does not. So always die on errors and reserve exit for clean exits when nothing has gone wrong.
3) finally as you can see from above i use the same variable names in both main and the subroutine but use my to keep them separate. I can see how that might confuse someone else looking at the code later and also if I forget to use my how it might confuse me :) I just keep running out of names. Is there a good standard to follow that others have found works ?I try to avoid reusing the same variable name for different things, since that has a chance to cause confusion.
But I have nothing against reusing variable names for the same thing in different scopes, which is what you seem to be doing here. (The sub's $ticket gets returned and assigned to the other $ticket, so the name $ticket always refers to the same data, even if it's actually two different variables.)
In reply to Re: subroutine and good practices
by dsheroh
in thread subroutine and good practices
by jmsnet
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |