2-liner-exception-messages-fix #3535
Conversation
KatKatKateryna
changed the title
| @@ -122,7 +122,7 @@ public ACG.GeometryType GetLayerGeometryType(VectorLayer target) | |||
| GISLayerGeometryType.POLYLINE => ACG.GeometryType.Polyline, | |||
| GISLayerGeometryType.MULTIPATCH => ACG.GeometryType.Multipatch, | |||
| GISLayerGeometryType.POLYGON3D => ACG.GeometryType.Multipatch, | |||
| _ => throw new ArgumentOutOfRangeException(nameof(target)), | |||
| _ => throw new ArgumentOutOfRangeException($"{originalGeomType}"), | |||
There was a problem hiding this comment.
What is the first parameter of ArgumentOutOfRangeException is it a message, or is it an argument name?
There was a problem hiding this comment.
It looks like it is wanting the parameter name, so nameof(target) was correct no?
You can allways do
new ArgumentOutOfRangeException(nameof(target), "my custom message about what this exception is...");There was a problem hiding this comment.
hmm, true, but parameter name doesn't help the user (e.g. message "System.ArgumentOutOfRangeException: 'Specified argument was out of the range of valid values. (Parameter 'MultiPolygonZ')'" is more helpful than "Parameter 'target' " / "Parameter 'originalGeomType' "
There was a problem hiding this comment.
It can also be $"{nameof(target)} = {originalGeomType}"
There was a problem hiding this comment.
We have to have info for both so can we include both in the message and use nameof() in the code, it might be a detail but if we're looking at this exception I want to know which thing in the code was out of range. I also don't have the context, so is the argument out of range or invalid?
There was a problem hiding this comment.
@BovineOx I eventually added both!
|
Thanks @oguzhankoral , now need @JR-Morgan to approve requested changes and unblock merge 🚀 |
BovineOx
dismissed
JR-Morgan’s stale review
Jedd is off and we've resolved the concern
Description & motivation
Changes:
To-do before merge:
Screenshots:
Validation of changes:
Checklist:
References