利用BeautifulSoup跟踪刮除月食/日食数据

Question

我决定基于这个评议重构我的代码。

Eclipse：

class Eclipse():

    def __init__(self, eclipse_date, eclipse_type, eclipse_location):
        self._validate_str_parameters(eclipse_date, "Eclipse date")
        self._validate_str_parameters(eclipse_type, "Eclipse type")
        self._validate_lst_parameters(eclipse_location, "Location")

        self._eclipse_date = eclipse_date
        self._eclipse_type = eclipse_type
        self._eclipse_location = eclipse_location


    def _validate_str_parameters(self, str_to_validate, parameter_type):
       if not isinstance(str_to_validate, str):
           raise TypeError("{0} is the incorrect type".format(parameter_type))
       if not str_to_validate:
           raise ValueError("{0} is empty".format(parameter_type))     

    def _validate_lst_parameters(self, lst_to_validate, parameter_type):
        if not isinstance(lst_to_validate, list):
            raise TypeError("{0} type is incorrect".format(parameter_type))
        if len(lst_to_validate) == 0:
            raise ValueError("{0} is invalid. You must have at least one.".format(parameter_type))

    @property
    def get_eclipse_date(self):
        return self._eclipse_date

    @property
    def get_eclipse_type(self):
        return self._eclipse_type

    @property
    def get_eclipse_location(self):
        return self._eclipse_location

    def date_and_type(self):
        return "{0}\n{1}\n".format(self._eclipse_date, self._eclipse_type)

    def month_and_year_is(self, eclipse_month, eclipse_year):
        self._validate_str_parameters(eclipse_month, "Month")
        self._validate_str_parameters(eclipse_year, "Year")
        return eclipse_month and eclipse_year in self._eclipse_date

    def takesplacein(self, country):
       self._validate_str_parameters(country, "Country")

       country_formatted = country.strip().title()
       muchof = "Much of {0}".format(country_formatted)
       return self._is_muchof_in_list(muchof) or self._is_country_in_list(country_formatted)

    def _is_muchof_in_list(self, muchofcountry):
        return muchofcountry in self._eclipse_location

    def _is_country_in_list(self, countryformatted):
        return countryformatted in self._eclipse_location

    def __str__(self):
        return "Date: {0}\nType: {1}\nLocation: {2}\n" .format(self._eclipse_date,self._eclipse_type,self._eclipse_location)

说明：我创建这个类是根据我从这个月食站点收集的数据创建对象。

抓取后的示例数据：

Date: July 13, 2018 
Type: Partial Solar Eclipse
Location: ['South in Australia', ' Pacific', ' Indian Ocean']

Date: July 27, 2018 
Type: Total Lunar Eclipse
Location: ['Much of Europe', ' Much of Asia', ' Australia', ' Africa', ' South in North America', ' South America', ' Pacific', ' Atlantic', ' Indian Ocean', ' Antarctica']

方法takesplacein(self, country)采用country (例如，欧洲)，并使用muchof = "Much of {0}".format(country_formatted)创建短语“大部分欧洲”。

该网站使用这个短语时，并不是所有的国家都会看到它，没有测试“大部分插入国家在这里”，我们只是测试国名，但它将返回False，即使国家的部分地区将看到它。

该方法测试“大部分插入国家”或国家本身，因为国家只能出现一次。它永远不会说欧洲和大部分欧洲。

Eclipse关注：

1. 方法名称takesplacein(self, country)和month_and_year_is(self, month, year)是否是干净的？我个人喜欢这些名字，因为这样我就可以编写听起来几乎像句子的代码，例如: if eclipse.takesplacein("europe")：print(eclipse.date_and_type()) if eclipse.month_and_year_is(“7月”，"2018")：print(eclipse.date_and_type())
2. 在这里使用isinstance()是合理的吗？我知道使用该方法或关键字被认为是代码嗅觉，但是没有它，当代码使用位置作为list时，我可以传递一个D16作为位置

主要：

import requests
from bs4 import BeautifulSoup
from Eclipse import Eclipse
from collections import namedtuple


def generate_eclipse_data(eclipse_info):

    eclipse_list = []

    for info in eclipse_info:
        retrieved_date_and_type = parse_date_and_type(info.find('a').text)
        retrieved_location_data = parse_location(info.find('p').text)
        eclipse_data = Eclipse(retrieved_date_and_type.date,retrieved_date_and_type.type, retrieved_location_data.location)
        eclipse_list.append(eclipse_data)
    return eclipse_list

def parse_location(info_to_clean):
    clean_location = namedtuple("Clean_Location",["location"])
    cleanup_location_and_new_features = info_to_clean.split(" New Features")
    cleanup_location, *rest, new_features = cleanup_location_and_new_features
    location_list = cleanup_location.split(",")
    location_data = clean_location(location_list)
    return location_data

def parse_date_and_type(info_to_split):
    date_and_type = namedtuple("Date_Type", ["date", "type"])
    eclipse_split_data = info_to_split.split("—")
    eclipse_date, *rest, eclipse_type = eclipse_split_data
    eclipse_data = date_and_type(eclipse_date,eclipse_type.strip())
    return eclipse_data

def main():

    eclipse_url = "https://www.timeanddate.com/eclipse/"
    eclipse_time_date = requests.get(eclipse_url)
    soup = BeautifulSoup(eclipse_time_date.text, 'html.parser')
    eclipse_info = soup.find_all("div", class_= "six columns art__eclipse-txt")
    eclipse_filename = 'eclipse.txt'

    if (len(eclipse_info) > 0):
        try:
            print("URL: {0}\n".format(eclipse_url))
            eclipse_data =  generate_eclipse_data(eclipse_info)

            for eclipse in eclipse_data:
                if eclipse.takesplacein("europe"):
                    print(eclipse.date_and_type())

            for eclipse in eclipse_data:
                if eclipse.month_and_year_is("July", "2018"):
                    print(eclipse.date_and_type())
        except ValueError as error:
            print(error)
        except TypeError as error:
            print(error)
        except Exception as error:
            print(error)
    else:
        print("No data available")

if __name__ == "__main__":
    main()

def parse_location(info_to_clean):构造list并将其存储在namedtuple中。

有什么办法可以进一步改进吗？

Answer 1

对isinstance()的使用是否合理？

您试图用isinstance()验证的部分内容是日期字符串。我建议通过转换和使用datetime.date对象来更好地完成这一切。这意味着在转换完成后可能会进行验证。至少就日期而言，这将消除对isinstance()的需求。

是takesplacein和month_and_year_is clean?

方法名。

我建议takesplacein更好地阅读，就像takes_place_in和month_and_year_is更好地阅读is_month_and_year一样。但更重要的一点是，我也更喜欢这类名称，因为正如您注意到的那样，它们可以更好地阅读代码。

属性并不真正需要_get__

这些属性使用get_前缀命名。就属性而言，它返回某些内容的事实相当明显，因此get_没有真正提供更多的信息。所以：

@property
def get_eclipse_date(self):
    return self._eclipse_date

@property
def get_eclipse_type(self):
    return self._eclipse_type

会更干净一点，因为简单地说：

@property
def eclipse_date(self):
    return self._eclipse_date

@property
def eclipse_type(self):
    return self._eclipse_type        

此外，我还考虑将其作为一项财产：

def date_and_type(self):
    return "{0}\n{1}\n".format(self._eclipse_date, self._eclipse_type)

Answer 2

此评审仅针对Main中的代码。一些风格上的建议，没有特别的顺序：

中间变量

中间产品的单独变量名称在一些情况下是有用的，例如：

1. 变量的名称对于记录中间产品所代表的内容非常重要。
2. 需要将一个非常复杂的表达式分解成较小的部分。
3. 中间产品在多个地方使用。

所以，我建议：

cleanup_location_and_new_features = info_to_clean.split("? New Features")
cleanup_location, *rest, new_features = cleanup_location_and_new_features
location_list = cleanup_location.split(",")
location_data = clean_location(location_list)

实际上更容易理解为：

cleanup_location, *rest, new_features = info_to_clean.split("? New Features")
location_data = clean_location(cleanup_location.split(","))

我不认为这两个中间变量名称提供了更多的清晰度或功能，但它们确实是代码行数的两倍。

多个异常类型：

这是：

except ValueError as error:
    print(error)
except TypeError as error:
    print(error)
except Exception as error:
    print(error)

可减为：

except (ValueError, TypeError, Exception) as error:
    print(error)

但是，由于ValueError和TypeError都是基于Exception的，因此可以进一步简化为：

except Exception as error:
    print(error)

但是捕获所有异常都被认为是不良做法，所以您可能需要考虑这里真正想要的功能。

删除冗余部分：

if (len(eclipse_info) > 0):

可以简单地说：

if len(eclipse_info) > 0:

任何> 0都会评估为True，所以您可以这样做：

if len(eclipse_info):

最后，如果它们的len是> 0，那么许多类型的计算结果为True，对于这些类型，您可以简单地这样做：

if eclipse_info:

PEP8

您应该考虑按照pep8对代码进行格式化。在共享代码时，这一点很重要，因为一致的样式使其他程序员更容易阅读您的代码。有各种工具可以帮助使代码pep8兼容。我使用的是PyCharm IDE，它将在编辑器中显示pep8违规情况。