Python - Советы по переписыванию функции BeautifulSoup, чтобы быть более элегантным - PullRequest
Новая
       23

Python - Советы по переписыванию функции BeautifulSoup, чтобы быть более элегантным

2 голосов
/ 27 октября 2011

Код работает, но я ищу советы о том, как это можно было бы написать немного более правильно, особенно использование if. Как вы можете заметить, я не программист по натуре ... просто системный администратор, немного играющий на python. Спасибо за любой совет, который вы можете дать. 

def findallWileyLinks():

pagebase = 'http://onlinelibrary.wiley.com'
journallist = 'http://onlinelibrary.wiley.com/browse/publications?type=journal&&start=0&resultsPerPage=3000'

inputList = getinputList()

if inputList:
  alljournallistsoup = BeautifulSoup(getwebpage(journallist))

  if alljournallistsoup:
    alljournallisttags = alljournallistsoup.find('ol', attrs={'id' : 'publications'})

    for eissn in inputList:
      journalatag = alljournallisttags.find('a', attrs={'href' : re.compile(eissn.rstrip() + '$')})

      if journalatag:
        journalsoup = BeautifulSoup(getwebpage(pagebase + journalatag.get('href') + '/issues'))

        if journalsoup:
          allvolumetags = journalsoup.find('ol', attrs={'class' : 'issueVolumes'})
          volumeatags = allvolumetags.findAll('a')

            for volumeatag in volumeatags:
              volumesoup = BeautifulSoup(getwebpage(pagebase + volumeatag.get('href')))

              if volumesoup:
                allissuetags = volumesoup.find('li', attrs={'id' : volumeatag.get('id')[:-5]})
                issueatags = allissuetags.findAll('a')[1:]

                  for issueatag in issueatags:
                    currentlinksavailiable.append(pagebase + issueatag.get('href') + '\n')

      else:
        appendlog('eISSN: ' + eissn.rstrip() + ' not found on alljournallist page.')

    try:
      with open(inputDirectory + selectedPublisher + '_currentlinksavailiable.txt', 'w') as f:
          f.writelines(currentlinksavailiable)

    except IOError as e:
      appendlog('findallLinks() Operation failed probably when creating the new link text file with error: %s' % e.strerror)

Ответы [ 4 ]

3 голосов
/ 27 октября 2011

Одна вещь, которая бросается в глаза, это то, что у вас много кода вида: 

tags = parenttag.findAll('tag')

if tags:
    for tag in tags:
        # do something to tag

Вам гарантировано, что tags - это список здесь, поэтому строка if tags:излишний.Если вы используете пустой список в цикле for, то тело цикла не выполняется.

В качестве маленькой точки, в начале файла inputList = [] можно удалить, так как вы сразу же перезаписаете егос вызовом функции.

Непонятно, является ли это частью большого скрипта или нет, но если это не так, вместо того, чтобы включать тело скрипта в блок if, вы должны прекратить, еслиinputList пусто. 

if not inputList:
    sys.exit(1)

вместо 

if inputList:
    # process inputList

Вам нужно будет добавить import sys в начало вашего скрипта, чтобы это работало.

1 голос
/ 27 октября 2011

Вы можете поместить код в функцию и сделать большую часть ваших операторов if отрицательными и использовать операторы return, continue или break.Это позволит избежать большого количества отступов.

Кроме того, перед циклом for есть оператор if, который выглядит так, как будто он не нужен, поскольку, если список пуст, цикл for пропускается.Т.е. 

if volumeatags:
    for volumeatag in volumeatags:
            ...
0 голосов
/ 27 октября 2011

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

if foo:
    if bar:
        #code

Заменить на (если вы внутри def, но вы, не так ли? 

if not foo:
    return
if bar:
    #code

Комментарии должны начинаться с hash-blank-uppercase, оставляйте hash-lowercase для кода, который вы закомментировали. Питер Норвиг, кажется, вставил две черточки в текстовые комментарии. В любом случае у вас слишком много комментариев. Код должен быть достаточно описательным. Если блок операций нуждается в дескриптивном заголовке, переместите его в функцию и используйте это описание в качестве имени функции. Это также поможет изолировать вещи.

Не охраняйте свои for foo петли с помощью if foo, как отмечено в других ответах.

inputList = [] бесполезен и не соответствует PEP08, попробуйте input.

0 голосов
/ 27 октября 2011

Большая часть этого кода не нужна.Некоторые примеры: 

inputList = []
inputList = getinputList()

В Python вам не нужно инициализировать переменные перед назначением им данных.Вторая строка будет в порядке сама по себе. 

if volumeatags:
   for volumeatag in volumeatags:

Если volumeatags пуст, цикл for не будет выполнен.

...