Моя первая программа на Python: можешь сказать, что я делаю не так? - PullRequest
6 голосов
/ 04 апреля 2009

Надеюсь, этот вопрос считается подходящим для stackoverflow. Если нет, я удалю вопрос сразу.

Я только что написал свою самую первую программу на Python. Идея состоит в том, что вы можете выполнить команду, и она будет отправлена ​​на несколько серверов параллельно.

Это только для личных образовательных целей. Программа работает! Я действительно хочу стать лучше на python, и поэтому я хотел бы задать следующие вопросы:

  1. Мой стиль выглядит грязным по сравнению с PHP (к чему я привык). У вас есть предложения по улучшению стиля?
  2. Использую ли я правильные библиотеки? Я правильно их использую?
  3. Использую ли я правильные типы данных? Я правильно их использую?

У меня хороший опыт программирования, но мне потребовалось много времени, чтобы разработать достойный стиль для PHP (стандарты PEAR-кодирования, зная, какие инструменты использовать и когда).

Источник (один файл, 92 строки кода)

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

Ответы [ 4 ]

10 голосов
/ 04 апреля 2009

Обычно предпочтительно, чтобы то, что следует после конца предложения :, было в отдельной строке (также не добавляйте пробел перед ним)

if options.verbose:
  print ""

вместо

if options.verbose : print ""

Вам не нужно проверять длину списка, если вы собираетесь его перебирать

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

является избыточным, более «читабельным» является (python достаточно умен, чтобы не перебирать пустой список):

for server in threadlist:
  ...

Также более «pythonistic» заключается в использовании постижений Листа (но, конечно, это спорно мнение)

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

можно сократить до

server = [getServers(i) for i in grouplist]
7 голосов
/ 04 апреля 2009

Прежде чем выгрузить любую критику, сначала позвольте мне поздравить вас с работой вашей первой программы на Python. Переход с одного языка на другой может быть непростой задачей, постоянно возиться с проблемами синтаксиса и охотиться через незнакомые библиотеки.

Наиболее цитируемое руководство по стилю: PEP-8 , но это только руководство, и, по крайней мере, некоторая его часть игнорируется ... нет, я имею в виду, что оно не применимо к какой-то конкретной ситуации со всеми должное уважение к авторам и авторам руководства: -).

Я не могу сравнить его с PHP, но по сравнению с другими приложениями Python довольно ясно, что вы следуете стилевым соглашениям других языков. Я не всегда соглашался со многими вещами, которые другие разработчики говорили, что вы должны делать, но со временем я понял, почему использование соглашений помогает понять, что делает приложение, и поможет другим разработчикам помочь вам.


Поднимайте исключения, а не строки.
raise 'Server or group ' + sectionname + ' not found in ' + configfile

становится

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


Не ставьте пробел перед ':' в конце 'if' или 'for', и не помещайте несколько операторов в одну строку, и будьте последовательны в отношении размещения пробелов вокруг операторов. Используйте имена переменных для объектов и придерживайтесь i и j для переменных индекса цикла (как наши предки FORTRAN):
for i in grouplist : servers+=getServers(i)

становится:

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


Контейнеры могут быть проверены на содержание без определения их длины:
while len(threadlist) > 0 :

становится

while threadlist:

и

if command.strip() == "" :

становится

if command.strip():


Разделение кортежа обычно не ставится в круглых скобках с левой стороны оператора, а логика команды немного запутана. Если аргументов нет, то "" .join (...) будет пустой строкой:
(options,args) = parser.parse_args()

if options.verbose : print "floep 0.1" 

command = " ".join(args)

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

становится

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

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


В цикле Python for есть необычное предложение 'else', которое выполняется, если цикл проходит через все элементы без 'break':
    for server in threadlist :
        foundOne = False 
        if not server.isAlive() :
            ...snip...
            foundOne = True

        if not foundOne :
            time.sleep(0.010)

становится * * тысяча пятьдесят-одна

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


Получение списка строк и последующее их объединение немного затянуто:
        result = proc.readlines()
        strresult = ''
        for line in result : strresult+=line 
        self.result = strresult

становится

        self.result = proc.read()


Вы хорошо используете библиотеку, проверьте модуль подпроцесса, он немного обновлен.

С вашими типами данных все в порядке.

И вы получите много других ответов: -)

5 голосов
/ 04 апреля 2009

Строковые исключения устарели в Python, поэтому эта строка:

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

должно быть переработано во что-то вроде этого:

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

class ConfigNotFoundError(Exception):
    pass

[Отредактировано, чтобы отразить предложение dangph в комментариях]

Это больше строк кода, но это лучше для будущих обновлений.

Для удобства чтения, что-то вроде этого:

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

Можно переписать так:

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

Вы могли бы предпочесть другой метод разделения вызова метода, но идея заключается в том, что длинные строки могут быть трудно читать.

Вы также должны прочитать PEP 8 , чтобы получить представление о стиле Python.

3 голосов
/ 04 апреля 2009

Часто для целей повторного использования мы делаем следующее, начиная примерно со строки 48 в вашей программе

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

if __name__ == "__main__":
    main()

Это только отправная точка.

Как только вы это сделаете, вы поймете, что 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., ... )

Теперь вы подняли реальную работу до функции в этом модуле. Теперь ваша функция serverWork может быть легко использована другими программами или скриптами.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...