The _device_data and create_* functions replace errors with print statements, then silently communicate the error to the caller by passing None. This puts the onus on the caller to handle the case where they get a None result. When this pattern repeats itself a None result can get kicked up two or three callers above the source of the error before someone has to make a decision.
See for example this pattern in _device_data:
try:
location = _find_yaml_file(
area=area,
)
with open(location, "r") as device_file:
device_data = yaml.safe_load(device_file)
if device_type:
if name:
return device_data[device_type][name]
return {device_type: device_data[device_type]}
return device_data
except FileNotFoundError:
print(f"Could not find yaml file for area: {area}")
return None
except KeyError as ke:
if ke.args[0] == device_type:
print("Device type ", device_type, " not supported.")
return None
if ke.args[0] == name:
...
This function gets called in create_magnet:
device_data = _device_data(area=area, device_type="magnets", name=name)
if not device_data:
return None
Such that the caller of create_magnet needs to know what to do with the None result.
If we're using None as a failure case, I would recommend we let the error happen instead. We can a more detailed error where it is called for, as in the case of _device_data. If we use errors we won't have to communicate the failure upwards through callers, which will reduce redundant code and make the usage of these functions more intuitive.
The
_device_dataandcreate_*functions replace errors with print statements, then silently communicate the error to the caller by passingNone. This puts the onus on the caller to handle the case where they get aNoneresult. When this pattern repeats itself aNoneresult can get kicked up two or three callers above the source of the error before someone has to make a decision.See for example this pattern in
_device_data:This function gets called in
create_magnet:Such that the caller of
create_magnetneeds to know what to do with theNoneresult.If we're using
Noneas a failure case, I would recommend we let the error happen instead. We can a more detailed error where it is called for, as in the case of_device_data. If we use errors we won't have to communicate the failure upwards through callers, which will reduce redundant code and make the usage of these functions more intuitive.