3 ms·
Try using "AskHN: " or "ShowHN: " to get more appropriate traction. Use a more mainstream Python naming scheme. I expect "Parser" to be a class name, not a fun
by eesmith 3y ago
Try using "AskHN: " or "ShowHN: " to get more appropriate traction.
Use a more mainstream Python naming scheme. I expect "Parser" to be a class name, not a function name. Most Python code uses underscore_names rather than camelCase.
Use subprocess.run() or subprocess.check_output() instead of writing your own variant using a pipe. You are supposed to wait on the process before closing, though garbage collection does that for you here.
Avoid the extra overhead of "shell=True" since you can just write ["prog", "opt"] or "prog opt".split().
Consider using subclasses like MacPlatform, LinuxPlatform, and BSDPlatform so you don't need the "if self.platform" in each method.
This would also make your methods smaller and more focused.
A downside is it's harder to re-use implementations for different classes. I generally use a mixin for that, for example, "_uname_m" might implement arch() for those OSes,
class _uname_m:
def arch(self): return subprocess.run(["uname", "-m"])
class MacPlatform(_uname_m, Platform):
...
You may consider hard-coding path names like /usr/bin/arch for the OS, to prevent path issues.
This:
if arches[arch] != arch:
return arches[arch]
return arch
can be replaced with
arches.get(arch, arch)
A related package is https://github.com/workhorsy/py-cpuinfo https://github.com/workhorsy/py-cpuinfo . You might look at it to see how they organize platform differences.
- traman124 3y agoThanks so much! My code quality system has been sending me emails every time I commit because of that shell=true thing, I think it's about time to change it. I'll switch to a better naming scheme if that makes my code more standard or easier to understand :D I also like your input on the architecture functions, although my Windows implementation is really bad and I know it (it's very alpha quality) (EDIT: That library you mentioned has some good ideas on fetching cpu info on Windows, whic has been a bit of a problem for me to implement) (EDIT 2: I have started to fix these issues)