Critic my module

[Alain Ketterlin]

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.