0

I was trying to solve a function problem in which I had to count the number of times digit 1 appeared in all non-negative integers less than n(given).

Here's my code:

int ones(int n, int d)
{
    int rounddown = n - n % pow(10, d+1);
    int roundup = rounddown + pow(10, d+1);
    int right = n % pow(10, d);
    int dig = (n/pow(10, d)) % 10;
    if(dig<1)
        return rounddown/10;
    else if(dig==1)
        return rounddown/10 + right + 1;
    else
        return roundup/10;
}

int countDigitOne(int n) {

    int count = 0;
    string s = to_string(n);
    for(int i=0;i<s.length();i++)
    count+=ones(n, i);   
    return count;        
}

But the following compilation error shows up:

Line 3: invalid operands of types '__gnu_cxx::__promote_2::__type {aka double}' and 'int' to binary 'operator%'

geedee
  • 45
  • 7
  • For digit manipulation programs, consider treating the number as a string of characters. You can convert a character digit to a numeric one by: `numeric_digit = character_digit - '0';`. – Thomas Matthews Jun 22 '18 at 02:24
  • 2
    It's trying to use `pow(double, double)` rather than what I assume you want, which is `pow(int,int)`. There are some solutions at https://stackoverflow.com/questions/101439/the-most-efficient-way-to-implement-an-integer-based-power-function-powint-int for how to add your own `powi(int,int)` – Tyler V Jun 22 '18 at 02:35
  • 4
    Aside all the simple given answers regarding the type problem, using `pow` with integer math is a recipe for pain, [because floating point math is broken](https://stackoverflow.com/questions/588004/is-floating-point-math-broken). You need to find some other way to do this without using `pow()`. End of story. – Sam Varshavchik Jun 22 '18 at 02:44
  • @SamVarshavchik Exactly. Casting `(int)pow(x,y)` as many answers suggest is asking for trouble when the result it returns is something like `8.99999999999999` and you get `8` rather than `9`. No reason to do floating point math for an integer problem. – Tyler V Jun 22 '18 at 02:52
  • @geedee thanks for accepting my answer. I also added a few more examples that might help you to find other / more performant solutions to your problem. – Alex Jun 22 '18 at 08:17

2 Answers2

2

The main problem is type conversion. The result of pow is double. The modulo operator does not work for double. You need to take fmod.

Fix your line 3 like this:

int rounddown = (int)(n - fmod(n, pow(10, d +1));

Because your values are all in the domain of integer you can also use:

int rounddown = n - n % (int)(pow(10, d + 1));

as suggested by others.


Just for completeness ... If you are not forced to use an arithmetic approach you can just char compare:

#include<iostream>
#include<string>

using namespace std;

int countOnesInNumber(string s)
{
    int res = 0;
    for(int i = 0; i < s.length(); i++)
    {
        if(s[i] == '1')
        {
            res++;
        }
    }
    return res;
}

long countOnes(int upper)
{
    long result = 0;
    for(int i = 0; i < upper; i++)
    {
        result += countOnesInNumber(std::to_string(i));
    }
    return result;
}

int main()
{
    string in;
    cout << "Please enter a number:";
    cin >> in;
    cout << endl;
    cout << "you choose:" << in << endl;
    int n = stoi(in);
    cout << "there are " << countOnes(n) << " ones under " << n << endl;

    cin.ignore();
    cin.get();
}

There is also a more sophisticated approach. The digits repeat for every magnitude. There is 1 one under 10, there are 10 times 1 one under 100 plus another 10 ones for the number 10...19. And so on. You can just calculate the number of ones under a given magnitude with:

int exp = (int)log10(n);
int ones = exp * (int)pow(10, exp - 1);

where n must be a magnitude of 10 (eg. 10, 100, 1000, 10000 ...). If you are good at maths you might even find a complete closed formula.

Alex
  • 1,857
  • 3
  • 36
  • 51
2

The function pow returns a double. The modulo operation is between integers, thus the error - you are trying to use the modulo between an int and a double. One way to solve it is to use casting:

int rounddown = n - n % (int)pow(10, d + 1);

As others said, using the pow function for integers as well as casting from double to int is inefficient and can lead to errors. I would suggest (by agreeing with Tyler V's suggestion) you to implement this simple recursive integer pow:

int ipow(int a, int b){
    if (b == 0)
        return 1;

    return a * ipow(a,b - 1);
}

This works for only for positive integers a,b but I think that for your purposes it is sufficient. Now you can write

int rounddown = n - n % ipow(10, d + 1);
yakobyd
  • 572
  • 4
  • 12
  • There are still a few minor bugs in your recursive solution. I'm sure the last line should be `a * ipow(a,b-1)`. Furthermore, your solution will run into an infinite loop for `b < 0`. You should add another condition `if(b < 0) return 0;` (because this would result in numbers `0 < x < 1` and will be `0` when cast to int. – Alex Jun 22 '18 at 06:20
  • You are right it should be `a*ipow(a,b-1)` (fixed it). As for `b<0` I have stated that this implementation assumes positive integers `a,b` (this is sufficient for the OP`s needs). – yakobyd Jun 22 '18 at 16:43