Skip to content
This repository has been archived by the owner on Sep 10, 2019. It is now read-only.

Clarify UIntBase error message #24

Merged
merged 2 commits into from
Jan 7, 2018
Merged

Clarify UIntBase error message #24

merged 2 commits into from
Jan 7, 2018

Conversation

ixje
Copy link
Member

@ixje ixje commented Jan 7, 2018

What current issue(s) does this address?, or what feature is it adding?
This one has been bothering me for a while so it's about time to clarify it. The error message when providing data that is not long enough is not self explanatory.

UInt256(data=bytearray.fromhex('aabb'))
UInt256(data=b'aa' * 32)

both produce

Invalid UInt

A more verbose error message can clarify what is wrong about the data.

How did you solve this problem?
make the error message more verbose.

Case 1

Invalid UInt - data length 2 != specified num_bytes 32

It's immediately clear I'm not providing enough data

Case 2

Invalid UInt - data length 64 != specified num_bytes 32

It becomes clear that the format I'm providing data in isn't valid and I have to convert it to raw bytes.

How did you make sure your solution works?
ran some manual tests in Ipython.

Did you add any tests?
no

Are there any special changes in the code that we should be aware of?
no

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 4726ce7 on ixje:clarify_uintbase_error into 191cca8 on CityOfZion:master.

Copy link
Contributor

@metachris metachris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I'd use Invalid UInt: data length instead of -.

Copy link
Collaborator

@localhuman localhuman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change message to Invalid UInt: data length ...

@coveralls
Copy link

coveralls commented Jan 7, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 4726ce7 on ixje:clarify_uintbase_error into 191cca8 on CityOfZion:master.

@localhuman localhuman merged commit 2e3f853 into CityOfZion:master Jan 7, 2018
@ixje ixje deleted the clarify_uintbase_error branch January 17, 2018 12:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants