我的第一个python程序：你能告诉我我做错了什么吗？

I hope this question is considered appropriate for *. If not, I'll remove the question right away.

我希望这个问题适合*。如果没有,我会马上删除这个问题。

I've just wrote my very first python program. The idea is that you can issue a command, and it's gets sent to several servers in parallel.

我刚刚编写了我的第一个python程序。我们的想法是你可以发出一个命令,并将它并行发送到多个服务器。

This is just for personal educational purposes. The program works! I really want to get better at python and therefore I'd like to ask the following questions:

这仅用于个人教育目的。该计划有效!我真的想在python上变得更好,因此我想问下列问题:

1. My style looks messy compared to PHP (what I'm used to). Do you have any suggestions around style improvements.

与PHP(我以前习惯)相比,我的风格看起来很混乱。你对款式改进有什么建议吗?

2. Am I using the correct libraries? Am I using them correctly?

我使用正确的库吗?我正确使用它们吗?

3. Am I using the correct datatypes? Am I using them correctly?

我使用正确的数据类型吗?我正确使用它们吗?

I have a good programming background, but it took me quite a while to develope a decent style for PHP (PEAR-coding standards, knowing what tools to use and when).

我有一个很好的编程背景,但我花了很长时间才开发出适合PHP的样式(PEAR编码标准,知道使用什么工具以及何时使用)。

The source (one file, 92 lines of code)

源码(一个文件,92行代码)

http://code.google.com/p/floep/source/browse/trunk/floep

4 个解决方案

#1

Usually is preferred that what follows after the end of sentence : is in a separate line (also don't add a space before it)

通常最好在句子结尾之后:在一个单独的行中(也不要在它之前添加空格)

if options.verbose:
  print ""

instead of

if options.verbose : print ""

You don't need to check the len of a list if you are going to iterate over it

如果要迭代它,则无需检查列表的len

if len(threadlist) > 0 : 
  for server in threadlist :
    ...

is redundant, a more 'readable' is (python is smart enough to not iterate over an empty list):

是多余的,更“可读”(python足够聪明,不能遍历空列表):

for server in threadlist:
  ...

Also a more 'pythonistic' is to use list's comprehensions (but certainly is a debatable opinion)

另外一个更“'pythonistic'是使用列表的理解(但肯定是一个有争议的意见)

server = []
for i in grouplist : servers+=getServers(i)

can be shortened to

可以缩短为

server = [getServers(i) for i in grouplist]

#2

Before unloading any criticism, first let me say congratulations on getting your first Python program working. Moving from one language to another can be a chore, constantly fumbling around with syntax issues and hunting through unfamiliar libraries.

在卸载任何批评之前,首先让我祝贺你的第一个Python程序正常运行。从一种语言转移到另一种语言可能是一件苦差事,不断摸索语法问题并通过不熟悉的库进行搜索。

The most quoted style guideline is PEP-8, but that's only a guide, and at least some part of it is ignored...no, I mean deemed not applicable to some specific situation with all due respect to the guideline authors and contributors :-).

最引用的风格指南是PEP-8,但这只是一个指南,至少有一部分被忽略......不,我的意思是认为不适用于某些特定情况,并对指南作者和贡献者给予应有的尊重: - )。

I can't compare it to PHP, but compared to other Python applications it is pretty clear that you are following style conventions from other languages. I didn't always agree with many things that other developers said you must do, but over time I recognized why using conventions helps communicate what the application is doing and will help other developers help you.

我无法将它与PHP进行比较,但与其他Python应用程序相比,很明显您遵循其他语言的样式约定。我并不总是同意其他开发人员说你必须做的很多事情,但随着时间的推移,我认识到为什么使用约定有助于传达应用程序正在做的事情,并将帮助其他开发人员帮助你。

---

Raise exceptions, not strings.

raise 'Server or group ' + sectionname + ' not found in ' + configfile

becomes

raise RuntimeError('Server or group ' + sectionname + ' not found in ' + configfile)

---

No space before the ':' at the end of an 'if' or 'for', and don't put multiple statements on the same line, and be consistent about putting spaces around operators. Use variable names for objects and stick with i and j for loop index variables (like our masterful FORTRAN forefathers):

for i in grouplist : servers+=getServers(i)

becomes:

for section in grouplist:
    servers += getServers(section)

---

Containers can be tested for contents without getting their length:

while len(threadlist) > 0 :

becomes

while threadlist:

and

if command.strip() == "" :

becomes

if command.strip():

---

Splitting a tuple is usually not put in parenthesis on the left hand side of a statement, and the command logic is a bit convoluted. If there are no args then the " ".join(...) is going to be an empty string:

(options,args) = parser.parse_args()

if options.verbose : print "floep 0.1" 

command = " ".join(args)

if command.strip() == "" : parser.error('no command given')

becomes

options, args = parser.parse_args()
if options.verbose:
    print "floep 0.1" 

if not args:
    parser.error('no command given')
command = " ".join(args)

---

A python for loop has an unusual 'else' clause which is executed if the loop goes through all of the elements without a 'break':

    for server in threadlist :
        foundOne = False 
        if not server.isAlive() :
            ...snip...
            foundOne = True

        if not foundOne :
            time.sleep(0.010)

becomes

    for server in threadlist:
        if not server.isAlive():
            ...snip...
            break
    else:
        time.sleep(0.010)

---

Getting a list of lines and then joining them back together is a bit long winded:

        result = proc.readlines()
        strresult = ''
        for line in result : strresult+=line 
        self.result = strresult

becomes

        self.result = proc.read()

---

Your library use is good, check out the subprocess module, it's a little more up-to-date.

Your datatypes are fine.

你的数据类型没问题。

And you'll get lots of other anwsers :-)

你会得到很多其他的东西:-)

#3

String exceptions are deprecated in Python, so this line:

在Python中不推荐使用字符串异常,因此这一行:

if not config.has_section(sectionname): 
    raise 'Server or group ' + sectionname + ' not found in ' + configfile

should be reworked into something like this:

应该重做这样的事情:

if not config.has_section(sectionname):
    raise ConfigNotFoundError(
        "Server or group" + sectionname + "not found in" + configfile)

class ConfigNotFoundError(Exception):
    pass

[Edited to reflect the suggestion of dangph in the comments]

[编辑在评论中反映了dangph的建议]

It's more lines of code, but it's better for future upgrades.

这是更多的代码行,但它对未来的升级更好。

For readability's sake, something like this:

为了便于阅读,可以这样:

parser.add_option('-q','--quiet',action="store_false", help="Display only server output", dest="verbose", default=True)

Can be rewritten like this:

可以像这样重写:

parser.add_option('-q',
                  '--quiet',
                  action="store_false",
                  help="Display only server output", 
                  dest="verbose", 
                  default=True)

You might prefer another method of splitting the method call up, but the idea is that long lines can be hard to read.

您可能更喜欢分割方法调用的另一种方法,但其想法是长行可能难以阅读。

You should also read PEP 8 to get a sense of Python style.

您还应该阅读PEP 8以了解Python风格。

#4

Often, for reuse purposes, we do the following, starting at about line 48 in your program

通常,出于重用目的,我们执行以下操作,从程序中的第48行开始

def main():
    config = ConfigParser.RawConfigParser()
    etc.

if __name__ == "__main__":
    main()

This is just a starting point.

这只是一个起点。

Once you've done this, you realize that main() is really two parts: parsing the command-line interface and doing the work. You then want to refactor things to look like this.

完成此操作后,您会发现main()实际上是两部分:解析命令行界面并完成工作。然后你想重构一下这样的东西。

def serverWork(group,...):
    servers = getServers(group)
    etc.

def main():
    config = ConfigParser.RawConfigParser()

    if command.strip() == "":
        parser.error('no command given')
    else:
        serverWork( options.group, options.etc., ... )

Now, you have elevated the real work to a function within this module. Your serverWork function can now be reused easily by other programs or scripts.

现在,您已将实际工作提升到此模块中的函数。您的serverWork功能现在可以被其他程序或脚本轻松重用。