-
-
Notifications
You must be signed in to change notification settings - Fork 416
Issue 4153 Changed code coverage so that all lines line up in the lst file. #830
Conversation
if (number < 1_000_000_000) | ||
return 9; | ||
|
||
if (number < 1_000_000_000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copy paste error, I believe it should be:
else
return 10;
See also: http://graphics.stanford.edu/~seander/bithacks.html#IntegerLog10Obvious
I suggest finding the maximum count first then getting the maximum digits from that. Then there is no performance concern and you can use a loop instead of the huge digits function. |
That is a good point. I'll make that change. |
I'll try to fix the test files this weekend or next week so that the auto-tester will pass. |
Now the test files will not have to be changed. |
You can simply use printf to achieve this formatting. |
Thank you for the example. I've integrated it into the code. |
@@ -230,6 +250,39 @@ shared static ~this() | |||
} | |||
} | |||
|
|||
uint digits( uint number ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uint digits( uint number)
{
if (number < 10)
return 1;
return digits(number / 10) + 1;
}
Auto-merge toggled on |
Auto-merge toggled off |
Actually please add a test case and squash those commits together but otherwise LGTM. |
All tests are in the dmd repository. If I create a test for this change it will need to be merged in to dmd repo, which will break the auto-tester till this code is merged. Is there any other way that this can be done? |
Auto-merge toggled on |
Issue 4153 Changed code coverage so that all lines line up in the lst file.
Yeah, I can just merge this first. |
https://issues.dlang.org/show_bug.cgi?id=4153