current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite

I have a class with a data member that needs to be rounded up to a 2 digit integer, irrespective of the number of the input digits.

For example:

roundUpto2digit(12356463) == 12 
roundUpto2digit(12547984) == 13 // The 5 rounds the 12 up to 13.

Currently my code looks like:

int roundUpto2digit(int cents){
    // convert cents to string
    string truncatedValue = to_string(cents);
    // take first two elements corresponding to the Most Sign. Bits
    // convert char to int, by -'0', multiply the first by 10 and sum the second 
    int totalsum =  int(truncatedValue[0]-'0')*10 + int(truncatedValue[1]-'0');
    // if the third element greater the five, increment the sum by one
    if (truncatedValue[2]>=5) totalsum++; 
    return totalsum;
}

How can this be made less ugly?

share|improve this question
    
You've got a bug in your code. You forgot the singlequotes on if (truncatedValue[2]>='5') totalsum++; –  Snowbody yesterday
2  
What if you have less than 2 digits initially? E.g. roundUpto2digit(1)? –  Aleksey Demakov yesterday
    
@ Aleksey Demakov that is a valid question. A line checking for length and if statement including only the MSB will do it. Thanks! –  simplicis veritatis yesterday
1  
What should be the result of roundUpto2digit(999)? Following your code the result will be 100. But this is a 3-digit integer. Is this okay or should it be further truncated to 10 to make a 2-digit integer? –  Aleksey Demakov yesterday
1  
What you may and may not do after receiving answers –  Jamal♦ yesterday

3 Answers 3

up vote 6 down vote accepted

If I understood your requirements correctly then it might be like this:

int roundUpto2digit(int cents) {
  if (cents < 100)
    return cents < 10 ? cents * 10 : cents;
  while ((cents + 5) >= 1000)
    cents /= 10;
  return (cents + 5) / 10;
}

The test:

#include <stdio.h>

void
test(int i) {
    printf("%d -> %d\n", i, roundUpto2digit(i));
}

int
main() {
    test(0);
    test(1);
    test(5);
    test(9);
    test(10);
    test(49);
    test(50);
    test(94);
    test(95);
    test(99);
    test(100);
    test(104);
    test(105);
    test(994);
    test(995);
    test(999);
    test(1000);
    test(1040);
    test(1050);
    return 0;
}

The result:

0 -> 0
1 -> 10
5 -> 50
9 -> 90
10 -> 10
49 -> 49
50 -> 50
94 -> 94
95 -> 95
99 -> 99
100 -> 10
104 -> 10
105 -> 11
994 -> 99
995 -> 10
999 -> 10
1000 -> 10
1040 -> 10
1050 -> 11

It is uncertain if [1, 9] range should map into [10, 90] (2 digits) or [1, 9] (1 digit). I could fix it, if the later case is true.

share|improve this answer
up vote 4 down vote

With some math you can avoid the string conversion.

First take the log10 of the input:

double lg10 = log10(input);

The integral part of that will contain the exponent while the fractional part will contain the digits of the mantissa.

Putting just the fractional part in the exponent of 10^exponent will create a number between 1 and 9.999 which is scaled by an exact power of ten. So we can multiply that number by 10 and round it for the result:

double frac = lg10 - floor(lg10); //note round to -inf to get the integral part
return (int)round(10*pow(10, frac));

This can overflow from 99.9 to 100 so we need to catch that.

Putting it all together results in:

int roundUpto2digit(int cents){
    if(cents == 0)
        return 0;

    double lg10 = log10(cents);
    double frac = lg10 - floor(lg10); 

    int result = (int)round(10*pow(10, frac));
    return result==100 ? 10 : result;
}
share|improve this answer
up vote 1 down vote

I don't know about making the code more elegant, but a faster way is to use a chain of ifs:

int mostSignificantThreeDigits(int value){
    if (value < 1000) return cents;
    if (value < 10000) return cents/10;
    if (value < 100000) return cents/100;
    // etc.
}

int nearestMultipleOf10Factor(int value){
    return (value+5)/10;
}

int truncatedToTwoDigits(int value){
    return nearestMultipleOf10Factor(mostSignificantThreeDigits(value));
}

It's also possible to use a loop for mostSignificantThreeDigits():

int mostSignificantThreeDigits(int value){
    while (value > 1000) {
      value /= 10;
    }
    return value;
}

but I suspect it's slower due to the repeated divisions.

int mostSignificantThreeDigits(int value){
    int factor = 1000;
    int divisor = 1;
    while (value > factor) {
      factor *= 10;
      divisor *= 10;
    }
    return value / divisor;
}

might be faster if integer multiplication is way faster than integer division.

share|improve this answer
    
The original code is undefined for values in the [0, 99] range. There will be out of bounds sting access in this case. Also there is ambiguity for values [995, 999]. The result will be 100, a 3-digit value, which contradicts the task description to get just 2 digits. With your version the result is the same in this case. For the former case your version will return a single-digit value. For instance, the return value for argument 50 will be 5. This is again contradicts the requirement to return always 2 digits. –  Aleksey Demakov yesterday

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.