Critic my module
Alain Ketterlin
alain at dpt-info.u-strasbg.fr
Thu Jul 25 10:09:15 EDT 2013
Devyn Collier Johnson <devyncjohnson at gmail.com> writes:
> I made a Python3 module that allows users to use certain Linux
> shell commands from Python3 more easily than using os.system(),
> subprocess.Popen(), or subprocess.getoutput(). This module (once
> placed with the other modules) can be used like this
Good, but I doubt it's really useful: I think nobody is going to add a
dependency on your module for, basically, one-line wrappers...
Here are a few comments:
> def ls():
> version = '0.3'
> print(subprocess.getoutput('ls'))
version is local here, so basically your first statement is useless
(search for "global" in python's language ref).
> def uname():
> version = '0.3'
> print(platform.uname())
I once learned: "never print anything in a library function". This is a
bad thing to do, for a variety of reasons. For instance, stdout may be
redirected during this call...
> def man(x):
> version = '0.3'
> print(subprocess.getoutput('man' + x))
getoutput is (essentially) Popen(...,shell=True), and the doc says:
"the use of shell=True is strongly discouraged in cases where the
command string is constructed from external input"
(for very good reasons)
> def clear_bash_history():
> version = '0.3'
> print(subprocess.getoutput('history -c'))
Who told you subprocess will use bash? Again, the doc:
"On Unix with shell=True, the shell defaults to /bin/sh."
All your uses of bash-isms may break (esp. "!!")
> def firefox():
> version = '0.3'
> print(subprocess.Popen('(firefox &)'))
See section "Replacing the os.spawn family" in... the doc.
> def go_back():
> version = '0.3'
> print(subprocess.Popen('cd !!:1'))
Hopeless. Have you tried this?
> def reboot():
> version = '0.3'
> print(subprocess.Popen('shutdown -r now'))
What do you expect this to print? I mean, after shutdown/reboot.
> version = '0.6b'
So, what's the version? 0.3 or 0.6b
(btw, are you sure this "version" is the same as the one you use in all
functions?).
-- Alain.
More information about the Python-list
mailing list