首页
学习
活动
专区
圈层
工具
发布
首页
学习
活动
专区
圈层
工具
MCP广场
社区首页 >问答首页 >板式切割机

板式切割机
EN

Code Review用户
提问于 2020-05-26 08:36:59
回答 2查看 529关注 0票数 6

因此,我刚刚完成了我的第一个Python模块(并发布在Github上),通过这个小项目,我想学习如何分发我的代码,以便其他用户可以将它作为自己项目的插件。

具体来说,我正在寻找以下方向的反馈:

  • 模块的接口设计正确吗?
  • 在代码开始时,我检查输入的完整性,这是处理错误的最佳方法吗?(看上去很粗壮)
  • 存储库的设置是否正确,以便它是即插即用的?
  • 一般来说,这是设计模块的最佳方式,还是应该使用类而不是函数?

任何其他反馈意见也欢迎:)

提前感谢!

指向Github存储库的链接:https://github.com/nick-van-h/cutlistcalculator

__main__.py:

代码语言:javascript
复制
from cutlist import getCutLists
import sys
import argparse

if __name__ == '__main__':
    #Argument parser
    text = "This program calculates the most optimal cutlist for beams and planks."
    parser = argparse.ArgumentParser(description=text)
    parser.add_argument("-i", "--input", help="custom location of input json file (e.g. 'localhost:8080/foo/bar.json'", default="")
    parser.add_argument("-o", "--output", help="custom location of output folder (e.g. 'localhost:8080/foo' -> 'localhost:8080/foo/cutlist_result.json'", default="")
    args = parser.parse_args()

    #Kick-off
    result = getCutLists(args.input, args.output)

    #Exit function with VS Code workaround
    try:
        sys.exit(result)
    except:
        print(result)

cutlist.py:

代码语言:javascript
复制
import json
from operator import itemgetter
import copy
from pathlib import Path
import os

def getSolution(reqs, combs):
    needs = copy.deepcopy(reqs)
    res = []
    res.append([])
    for comb in combs:
        #As long as all items from comb[x] fulfill need
        combNeed = True
        while combNeed:
            #Check if comb[x] provides more than need (fail fast)
            for need in needs:
                if comb[need['Length']] > need['Qty']:
                    combNeed = False
            if not combNeed:
                break

            for need in needs:
                need['Qty'] -= comb[need['Length']]

            #Append result
            res[0].append(comb.copy())

    #Calculate total price
    for sol in res:
        price = round(sum(x['Price'] for x in sol),2)

    res.append([price])

    #Return result
    return res

def getCutLists(inputstr = "", outputstr = ""):
    if inputstr:
        jsonlocation = inputstr
    else:
        jsonlocation = './input/input.json' #default input location
    print(jsonlocation)
    errstr = ""

    #Get input
    try:
        with open(jsonlocation) as f:
            data = json.load(f)
    except:
        errstr += "JSON file not found. "
        return(f"Err: {errstr}")

    #Get variables from JSON object
    try:
        reqs = data['Required Lengths']
    except:
        errstr += "'Required Lengths' not found. "

    try:
        avail = data['Available base material']
    except:
        errstr += "'Available base material' not found. "

    try:
        cutwidth = data['Cut loss']
    except:
        errstr += "'Cut loss' not found. "

    if errstr:
        return(f"Err: {errstr}")

    #Test for required keys in array
    try:
        test = [x['Length'] for x in reqs]
        if min(test) <= 0:
            errstr += f"Err: Required length ({min(test)}) must be bigger than 0."
    except:
        errstr += "'Length' not found in required lengths. "

    try:
        test = [x['Qty'] for x in reqs]
        if min(test) <= 0:
            errstr += f"Err: Required quantity ({min(test)}) must be bigger than 0."
    except:
        errstr += "'Qty' not found in required lengths. "

    try:
        test = [x['Length'] for x in avail]
        if min(test) <= 0:
            errstr += f"Err: Available length ({min(test)}) must be bigger than 0."
    except:
        errstr += "'Length' not found in available base material. "

    try:
        test = [x['Price'] for x in avail]
        if min(test) < 0:
            errstr += f"Err: Available price ({min(test)}) can't be negative."
    except:
        errstr += "'Price' not found in available base material. "

    if errstr:
        return(f"Err: {errstr}")


    #Init other vars
    listreq = [x['Length'] for x in reqs]
    listavail = [x['Length'] for x in avail]
    minreq = min(listreq)
    res=[]

    #Error handling on passed inputs
    if max(listreq) > max(listavail):
        return(f"Err: Unable to process, required length of {max(listreq)} is bigger than longest available base material with length of {max(listavail)}.")

    if cutwidth < 0:
        return(f"Err: Cut width can't be negative")

    #Make list of all available cut combinations
    combs = []
    for plank in avail:
        myplank = plank.copy()
        for cut in reqs:
            myplank[cut['Length']] = 0

        #Increase first required plank length
        myplank[reqs[0]['Length']] += 1

        #Set other variables
        myplank['Unitprice'] = myplank['Price'] / myplank['Length']

        filling = True
        while filling:
            #Calculate rest length
            myplank['Rest'] = myplank['Length']
            for i in reqs:
                length = i['Length']
                myplank['Rest'] -= ((myplank[length] * length) + (myplank[length] * cutwidth))
            myplank['Rest'] += cutwidth

            #Set rest of variables
            myplank['Baseprice'] = (myplank['Price']) / ((myplank['Length'] - myplank['Rest']))
            myplank['Optimal'] = (myplank['Rest'] <= minreq)


            #Check if rest length is positive
            if myplank['Rest'] >= 0:
                combs.append(myplank.copy())
                myplank[reqs[0]['Length']] += 1
            else:
                for i in range(len(reqs)):
                    if myplank[reqs[i]['Length']] > 0:
                        myplank[reqs[i]['Length']] = 0
                        if i < len(reqs)-1:
                            myplank[reqs[i+1]['Length']] += 1
                            break
                        else:
                            filling = False

    #Sort combinations descending by remaining length, get solution
    combs = sorted(combs, key=lambda k: k['Rest'])
    res.append(getSolution(reqs, combs))

    #Sort combinations by getting biggest lengths first (largest to smallest), optimal pieces first, get solution
    listreq = sorted(listreq, reverse=True)
    listreq.insert(0,'Optimal')
    for x in reversed(listreq):
        combs.sort(key=itemgetter(x), reverse=True)
    res.append(getSolution(reqs, combs))

    #Sort combination by least effective price per unit, get solution
    combs = sorted(combs, key=lambda k: k['Baseprice'])
    res.append(getSolution(reqs, combs))

    #Get cheapest option & make readable format
    cheapest = min([x[1] for x in res])
    for x in res:
        if x[1] == cheapest:
            sol = {}
            sol['Required base material'] = {}
            sol['Cut list'] = []
            i = 1
            for plank in x[0]:
                if plank['Length'] not in sol['Required base material']:
                    sol['Required base material'][plank['Length']] = 0
                sol['Required base material'][plank['Length']] += 1
                str = f"Plank {i}: Length {plank['Length']}, "
                for req in reqs:
                    if plank[req['Length']] > 0: str += f"{plank[req['Length']]}x {req['Length']}, "
                str += f"rest: {plank['Rest']}"
                sol['Cut list'].append(str)
                i += 1

            sol['Total price'] = cheapest
            break

    #Get output location
    if outputstr:
        outputfile = outputstr
        if outputfile[len(outputfile)-1] != "//":
            outputfile += "//"
        outputfile += "cutlist_result.json"
    else:
        outputfile = "./output/cutlist_result.json"

    #Make directories
    Path(os.path.dirname(outputfile)).mkdir(parents=True, exist_ok=True)

    #Output to file
    f = open(outputfile, "w")
    json.dump(sol, f, indent=4)
    f.close

    return("Success")
EN

回答 2

Code Review用户

回答已采纳

发布于 2020-05-26 12:53:30

地点

地点文档令人困惑。在getCutLists中,输入默认为

代码语言:javascript
复制
'./input/input.json'

但是在您的main中,文档中的示例是

代码语言:javascript
复制
'localhost:8080/foo/bar.json'

这是文件路径还是URL?根据您的使用情况,它看起来必须是一个文件路径,上面显示的主机和端口不应该在那里。另外,'./input/input.json'只应该是inputstr的默认值,而不是""

函数名

在Python中,函数和变量名的标准是lower_snake_case,即get_cut_listsget_solution等。

函数复杂度

对于可维护性、可测试性和可读性,getCutLists至少应该分为三个不同的功能。

异常处理

不要将异常降级为这样的字符串:

代码语言:javascript
复制
try:
    ...
except:
    errstr += "JSON file not found. "
    return(f"Err: {errstr}")

这种模式有几个问题。首先,except:干扰了用户突破程序的Ctrl+C能力。而且,except:在一般情况下过于宽泛,您应该只捕获您期望代码抛出的内容,在本例中是FileNotFoundError。此外,如果您希望您的错误字符串有用,您将包括文件的名称。最后,所有这些机器都应该消失,您应该简单地使用open(),让异常传递给没有except的调用者。如果调用者想要重新格式化在上层打印异常的方式,它可以;但这不应该是此函数的责任。在具有良好异常处理的语言中,要避免的一种模式是将异常处理降级为标量返回值(字符串、bool、int错误代码等)。

至于这样的验证:

代码语言:javascript
复制
try:
    test = [x['Length'] for x in reqs]
    if min(test) <= 0:
        errstr += f"Err: Required length ({min(test)}) must be bigger than 0."
except:
    errstr += "'Length' not found in required lengths. "

代之以提出你自己的例外:

代码语言:javascript
复制
min_len = min(x['Length'] for x in reqs)
if min_len <= 0:
    raise ValueError(f'Required length ({min_len}) must be greater than 0.')

此外,不要列出临时列表;直接将min应用于生成器。

注释

鉴于

代码语言:javascript
复制
#Make list of all available cut combinations

是个有用的评论,

代码语言:javascript
复制
#Set other variables

不是。这比没有任何评论更糟糕。如果有什么复杂或令人惊讶的事情发生,或与业务逻辑有关,记录它,否则避免

代码语言:javascript
复制
# do the thing
do_thing()

表达式简化

代码语言:javascript
复制
((myplank[length] * length) + (myplank[length] * cutwidth))

可以是

代码语言:javascript
复制
myplank[length]*(length + cut_width)

弱型结构

您是从JSON加载的;好的:但是您永远不会将数据的字典表示解压缩到对象中;而是将其放在字典中。这将导致代码,如

代码语言:javascript
复制
        myplank['Baseprice'] = (myplank['Price']) / ((myplank['Length'] - myplank['Rest']))

真是一团糟。相反,创建实际的类来表示数据,并将其解压缩到这些类中。

换句话说,我们不是在Javascript中:不是每件事都是字典。

混合操作系统/路径

代码语言:javascript
复制
Path(os.path.dirname(outputfile)).mkdir(parents=True, exist_ok=True)

使用混合Path (很好)和os调用(不太好)。这里不需要dirname;相反,直接使outputfile成为一条路径,然后对其进行操作。

票数 7
EN

Code Review用户

发布于 2020-05-26 19:38:48

源目录

将模块放在单独的源目录中。这样做的好处是可以使用pip install -e单独安装这个目录,或者将它添加到虚拟环境站点包中的.pth中。您正在使用虚拟环境进行开发吗?

tools

使用一个好的IDE和工具来改进您的代码。我使用black作为代码格式化程序,使用严格配置的mypy检查输入错误,使用pydocstyle检查我的文档字符串,使用pytest检查单元测试,使用pyflakes检查其他错误。了解它们,寻找大型python项目的配置灵感,并将它们集成到您的工作流中。大多数IDE让这件事变得很简单。

变量名

在python中,变量名的长度对程序的性能没有影响。然后选择清晰的变量名,如requested_planksreqs。由于这些不清楚的名称,代码的解码非常困难。

函数的

拆分(

)

您已经有了两个函数,但是这段代码需要更多的功能。

  • 读取输入
  • 验证输入
  • 使组合
  • 挑选一个组合
  • 输出到输出文件

其中每一项都应发挥自己的作用。这样做可以使您更好地记录这一点,测试不同的部分,并在未来进行更改。

我试着把我的功能分开,这样传输的数据就清楚了。

读取输入

提升IO (talks:1 2)不要传递输入文件。读取main()中的输入文件、函数,并将内容传递给验证器和以后的计算。输出也是如此。计算返回所需的板,然后如果需要,main()函数将结果写入磁盘。

验证输入

您的输入验证被分散在主方法上。您还可以使用字符串进行通信。另一种方法是将验证失败与ValueError通信。

如果您添加了类型提示和docstring,您可能会得到这样的结果:

代码语言:javascript
复制
import typing


class Plank(typing.NamedTuple):
    """Requested plank."""

    Length: float
    Qty: int


class BasePlank(typing.NamedTuple):
    """Available base plank."""

    Length: float
    Price: float  # or Decimal?


InputData = typing.TypedDict(
    InputData,
    {
        "Cut loss": float,
        "Required Lengths": typing.List[Plank],
        "Available base material": typing.List[BasePlank],
    },
)


def validate_planks(planks: typing.Iterable[Plank]) -> None:
    """Validate the requested planks.

    - Length must be larger than 0
    - Quantity must be larger than 0
    """
    for plank in planks:
        if "Length" not in plank:
            raise ValueError(f"`Length` not found in {plank}")
        if "Qty" not in plank:
            raise ValueError(f"`Qty` not found in {plank}")
        if plank["Length"] < 0:
            raise ValueError(f"`Length` < 0 in {plank}")
        if plank["Qty"] < 0:
            raise ValueError(f"`Qty` < 0 in {plank}")


def validate_baseplanks(planks: typing.Iterable[BasePlank],) -> None:
    """Validate the available base planks.

    - Length must be larger than 0
    - price must not be negative
    """
    for plank in planks:
        if "Length" not in plank:
            raise ValueError(f"`Length` not found in {plank}")
        if "Qty" not in plank:
            raise ValueError(f"`Qty` not found in {plank}")
        if plank["Length"] < 0:
            raise ValueError(f"`Length` < 0 in {plank}")
        if plank["Price"] <= 0:
            raise ValueError(f"negative `Price` in {plank}")


def validate_input(input_data: InputData) -> None:
    """Validate the input."""

    if "Cut loss" not in input_data:
        raise ValueError("`Cut loss` not found.")
    if "Available base material" not in input_data:
        raise ValueError("`Available base material` not found.")
    baseplanks = input_data["Available base material"]
    validate_baseplanks(baseplanks)

    if "Required Lengths" not in input_data:
        raise ValueError("`Required Lengths` not found.")
    planks = input_data["Required Lengths"]
    validate_planks(planks)

    if max(plank["Length"] for plank in planks) > max(
        plank[Length] for plank in baseplanks
    ):
        raise ValueError(
            "Maximum requested piece is longer than longest base plank"
        )

jsonschema

或者您可以使用jsonschema为您进行验证:

代码语言:javascript
复制
schema = jsonschema.Draft7Validator(
    {
        "type": "object",
        "properties": {
            "Cut loss": {"type": "number", "minimum": 0},
            "Required Lengths": {
                "type": "array",
                "items": {
                    "type": "object",
                    "properties": {
                        "Length": {"type": "number", "exclusiveMinimum": 0},
                        "Qty": {
                            "type": "number",
                            "exclusiveMinimum": 0,
                            "multipleOf": 1,
                        },
                    },
                    "required": ["Length", "Qty"],
                },
            },
            "Available base material": {
                "type": "array",
                "items": {
                    "type": "object",
                    "properties": {
                        "Length": {"type": "number", "exclusiveMinimum": 0},
                        "Price": {"type": "number", "minimum": 0},
                    },
                    "required": ["Length", "Price"],
                },
                "minProperties": 1,
            },
            "required": [
                "Cut loss",
                "Available base material",
                "Required Lengths",
            ],
        },
    }
)

然后使用

代码语言:javascript
复制
errors = list(schema.iter_errors(data))

验证了输入数据之后,您可以选择将它们放入类中,但是对于这个解决方案,这可能有点过了。

测试

这样,您可以单独测试验证。

在单独的目录tests、文件test_cutlist.py或要测试的每个函数的单独文件中。

代码语言:javascript
复制
import pytest

def test_validatbaseplanks():
    correct_data = [
        {
            "Length": 300,
            "Price": 5.95
        },
        {
            "Length": 180,
            "Price": 2.95
        },
        {
            "Length": 360,
            "Price": 6.95
        }
    ]
    cutlistcalculator.validate_baseplanks(correct_data)

    missing_price = [
        {
            "Length": 300,
        },
        {
            "Length": 180,
            "Price": 2.95
        },
        {
            "Length": 360,
            "Price": 6.95
        }
    ]
    with pytest.raises(ValueError) as excinfo:
        cutlistcalculator.validate_baseplanks(correct_data)
    assert "`Price` not found" in str(excinfo.value)

等等。

JSON

考虑一下要序列化输入和输出的格式。您使用JSON,但正如您注意到的,这有一些缺点。它非常冗长,您不能添加注释。JSON的意思是让计算机很容易读懂。替代方案有BSONTOML,.

我不是说这些更好,但至少看一看。特别是在开发的这么早的时候,很容易切换。

另一方面,如果正确划分代码,并使输入解析成为自己的功能,那么以后可以轻松地更改输入或输出格式。您甚至可以预见多个解析器,并接受不同的格式。

计算

我不懂你用的算法。我没有太多的时间去弄清楚它,但是你用不清楚的名字和把它都放在一个大的小块里的方式是没有帮助的。尝试将其划分为逻辑结构,然后重构为分离函数。仔细命名函数的名称,并预测一个docstring并键入hnts。一旦你有了它,把它们作为一个新的问题再发一次。

制作一个功能,产生可能的削减计划,只有必要的木板和可用的基板作为输入。让它成为一个发电机,产生一个可能的组合。您可以将其导入一个计算此安排成本的函数。这需要一个组合和基板的价格作为参数,并返回组合的成本。通过像这样划分工作,您可以记录它们的行为,并且可以分别测试这些组件。

输出

将其与计算最佳解决方案的代码分开。

使用with语句构造上下文。

代码语言:javascript
复制
with output_file.open("w") as filehandle:
    json.dump(filehandle, result, indent=2)

结论

我知道这是很多,但尝试合并这些技巧,从莱因德林,然后如果你不确定回来一个新的版本。继续做好工作

票数 3
EN
页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持
原文链接:

https://codereview.stackexchange.com/questions/242933

复制
相关文章

相似问题

领券
问题归档专栏文章快讯文章归档关键词归档开发者手册归档开发者手册 Section 归档