Beefy Boxes and Bandwidth Generously Provided by pair Networks
Welcome to the Monastery
 
PerlMonks  

Re: Defactor this code

by smahesh (Pilgrim)
on Feb 10, 2007 at 13:37 UTC ( #599355=note: print w/ replies, xml ) Need Help??


in reply to Defactor this code

Hi xerophyte,

I hope this is not homework. Some suggestions that I could quickly come up with are:

  • Use "use strict;" and "use warnings;" in the code (atleast during development).
  • Minor nitpick: Use a better named variable than "$errorstate". Using "$errorstate = 1" to indicate that valid sshport was input is confusing. I would have used a name like "$isvalidport", etc.
  • Instead of reading in the port value from <STDIN>, pass it as a cmdline argument.
  • Port numbers are limited to 65535 max. So, a value larger than that should be treated as invalid data.
  • Instead of using s/.../.../ to modify $_, why not print the new line "Port $sshport" to SSHD_CONFIG_NEW directly? I feel the substition operation is not required. The current print statement can be moved inside an else block.
  • Please close the opened file handles before calling rename(...)
  • You should check the return status of the rename(...) command.

Regards,
Mahesh


Comment on Re: Defactor this code

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://599355]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others chilling in the Monastery: (17)
As of 2015-07-31 12:33 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    The top three priorities of my open tasks are (in descending order of likelihood to be worked on) ...









    Results (277 votes), past polls