aboutcode-org / aboutcode-org/vulnerablecode

Limit 'try' block to a single piece of logic

未关闭
#876 0 条评论 0 个 reaction 已指派 1 人 已被 @johnmhoran 认领 在 GitHub 查看
ui
主要语言
Python
星标
702
派生
328
平均合并
3 天 8 小时
30 天内合并 PR
3

描述

From @tdruez 's comments in https://github.com/nexB/vulnerablecode/issues/875 .

Excerpts:

![image](https://user-images.githubusercontent.com/11096678/186770892-9e364ba3-5576-487b-ac54-986b95069040.png)

The QS should live outside the try block.

> what's the reason to move the Qs outside the try block

In general, you want to keep only one piece of logic that could fail within a `try` block.
An exception raised by converting a PURL has nothing to do with an exception filtering a QuerySet.

Also, using generic `except:` is bad practice. You should always try to be explicit about the exception you want to catch. Since you want to "Check whether the input value is a syntactically-correct purl", let's catch that specific Exception:
```
>>> PackageURL.from_string('wrong syntax')
Traceback (most recent call last):
File "", line 1, in
File "/Volumes/nexB/repos/scancode.io/lib/python3.9/site-packages/packageurl/__init__.py", line 354, in from_string
raise ValueError(
ValueError: purl is missing the required "pkg" scheme component: {repr(purl)}.
```

The packageurl raise a `ValueError` in that case, that's the exception we want to catch.
You code could become:

```
try:
purl = PackageURL.from_string(package_name)
except ValueError:
purl = None

if purl:
packages = ...
else:
packages = ...

return list(packages)
```

This way, this are properly grouped, the code is more readable:
1. we deal with the purl
2. we prepare a queryset based on the available data
3. we return the fetched objects

> keeping the try focused on its sole purpose: the purl test?

Yes, bottom line is to always try to keep you `try` block as focused as possible.

贡献指南

这个仓库没有索引到贡献指南

评估

这个 Issue 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。