CODEWITHSUNDEEP
c++
Tuilip Sharma
Tuilip Sharma

Reputation: 13

How to not repeat code in calculation functions?

I have made a simple program that performs basic arithmetic operations on all the elements of a given array. But the problem is that the code is very repetitive and it's not a good practice to write repeated code and I can't come up with a solution to this problem. How can we minimize code repetition in this program?

int add(int numcount, int numarr[]){
    int total = numarr[0];

    // add all the numbers in a array
    for (int i = 1; i < numcount; i++){
        total += numarr[i];
    }

    return total;
}

int sub(int numcount, int numarr[]) {
    int total = numarr[0];

    // subtract all the numbers in array
    for (int i = 1; i < numcount; i++){
        total -= numarr[i];
    }

    return total;
}

int mul(int numcount, int numarr[]) {
    int total = numarr[0];

    // multiply all the numbers in array
    for (int i = 1; i < numcount; i++){
        total *= numarr[i];
    }

    return total;
}

int main(int argc, char* argv[]){
    const int n = 5;
    int arr[n] = {1, 2, 3, 4, 5};
    
    cout << "Addition: " << add(n, arr) << endl;
    cout << "Subtraction: " << sub(n, arr) << endl;
    cout << "Multiplication: " << mul(n, arr) << endl;
}

Upvotes: 0

Views: 148

Answers (3)

463035818_is_not_an_ai
463035818_is_not_an_ai

Reputation: 122133

You can use std::accumulate:

 auto adder = [](auto accu,auto elem) { return accu + elem; };
 auto multiplier = [](auto accu, auto elem) { return accu * elem; };


 auto sum = std::accumulate(std::begin(arr),std::end(arr),0,adder);
 auto prod = std::accumulate(std::begin(arr),std::end(arr),0,multiplier);

The result of sub is just 2*arr[0] - sum.

Be careful with the inital value for std::accumulate. It determines the return type and multiplying lots of int can easily overflow, perhaps use 0LL rather than 0.

In cases where std::accumulate nor any other standard algorithm fits, and you find yourself writing very similar functions that only differ by one particular operation, you can refactor to pass a functor to one function that lets the caller specifiy what operation to apply:

template <typename F>
void foo(F f) {
    std::cout << f(42);
}
int identity(int x) { return x;}

int main() {
    foo([](int x) { return x;});
    foo(identity);    
}

Here foo prints to the console the result of calling some callable with parameter 42. main calls it once with a lambda expression and once with a free function.

Upvotes: 1

Useless
Useless

Reputation: 67713

How can we minimize code repetition in this program?

Generically, by identifying the repeated structure and seeing whether we can either abstract it out, or find an existing name for it.

  1. The repeated structure is just setting the running result to the first element of a container, and using a binary function to combine it with each subsequent element in turn.
  2. Take a look at the Standard Algorithms library and see if any existing function looks similar
  3. std::accumulate can do what we need, without any extra arguments for the add, and with just the appropriate operator function object for the others

So, you can trivially write

int add(int numcount, int numarr[]){
    return std::accumulate(numarr, numarr+numcount, 0);
}

// OK, your sub isn't actually trivial
int sub(int numcount, int numarr[]){
    return std::accumulate(numarr+1, numarr+numcount, numarr[0],
                           std::minus<int>{});
}

// could also be this - honestly it's a little hairy
// and making it behave well with an empty array
// requires an extra check. Yuck.
int sub2(int numcount, int numarr[]){
    return numarr[0] - add(numcount-1, numarr+1);
}

etc.

It would be slightly nicer to switch to using std::array, or to use ranges if you're allowed C++20 (to abstract iterator pairs over all containers).

If you must use C arrays (and they're not decaying to a pointer on their way through another function), you could write

template <std::size_t N>
int add(int (&numarr)[N]){
    return std::accumulate(numarr, numarr+N, 0);
}

to save a bit of boilerplate (passing numcount everywhere is just an opportunity to get it wrong).

NB. as mentioned in the linked docs, std::accumulate is an implementation of a left fold. So, if the standard library didn't provide accumulate, there's still an existing description of the "thing" (the particular "higher-order function") we're abstracting out of the original code, and you could write your own foldl template function taking the same std::plus, std::minus etc. operator functors.

Upvotes: 2

user12002570
user12002570

Reputation: 1

How can we minimize code repetition in this program?

One way to do this is to have a parameter of type char representing the operation that needs to be done as shown below:

//second parameter denotes the operator which can be +, - or *
int calc(int numcount, char oper, int numarr[])
{
    //do check here that we don't go out of bounds 
    assert(numcount > 0);
    
    int total = numarr[0]; 

    // do the operatations for all the numbers in the array
    for (int i = 1; i < numcount; i++){
        total = (oper == '-') * (total - numarr[i]) + 
                (oper == '+') * (total + numarr[i]) + 
                (oper == '*') * (total * numarr[i]); 
                 
    }
    return total;
           
}
int main()
{
    int arr[] = {1,2,3,4,5,6};
    std::cout << calc(6, '+', arr) << std::endl; //prints 21 
    std::cout << calc(6, '-', arr) << std::endl; //prints -19
    std::cout << calc(6, '*', arr) << std::endl; //prints 720
  
}

Working demo

Upvotes: -2

Related Questions