3
\$\begingroup\$

The program reads in an operation, and then numbers, and does stuff to them based on that.

Exercise number 1 at this linl

For example, ./this_file sum 1 2 3 would yield 6

I'm fairly certain I don't need all the #includes, but I had added them at some point to deal with a problem, some ideas I kept others I didn't.

I've also started reading Accelerated C++ and I realize that I should have probably tried to use things like Array<> if it's to really be C++.

/*
 * Write a program that takes as its first argument one of the words ‘sum,’ ‘product,’ ‘mean,’ or ‘sqrt’  and for further arguments a series of numbers. 
 * The program applies the appropriate function to  the series. 
 */

#include <stdio.h>
#include <math.h>
#include <stdlib.h>
#include <iostream>
#include <string.h>

using namespace std;

// Prototypes
double sum(int argc,const char* argv[]);
double product(int argc, const char* argv[]);
double mean(int argc, const char* argv[]);


int main(int argc, const char* argv[])
{
    double return_value;
    if(argc == 3)
    {
        //This means we're in sqrt, as we don't support sqrt of > 1 number
        //Nor sum, product, etc of less than one. 'cause that's dumb

        double operand;
        operand = strtod(argv[2], NULL );
        return_value = sqrt(operand);

    }
    else
    {   
        if(strcmp(argv[1],"sum") == 0)
        {
            return_value = sum(argc, argv);
        }
        if(strcmp(argv[1],"product") == 0)
        {
            return_value = product(argc, argv);
        }
        if(strcmp(argv[1],"mean") == 0)
        {
            return_value = mean(argc, argv);
        }
    }

    cout << return_value << endl;
}


double sum(int argc,const char* argv[])
{   

    double return_value = 0.0;
    for(int i = 2; i < argc; i++)
    {
        double x = strtod(argv[i], NULL); 
        return_value += x;
    }
    return return_value;
}

double product(int argc, const char* argv[])
{

    double return_value = 1;
    for(int i = 2; i < argc; i++)
    {
        return_value *= strtod(argv[i],NULL);   
    }
    return return_value;
}

double mean(int argc,const char* argv[])
{
    double summation = sum(argc, argv);
    return summation/((double) argc-2);
}
\$\endgroup\$
4
\$\begingroup\$
  • You're using C headers (which end in .h), not the C++ ones. This will also ensure that they're put into the std namespace. Also, you can remove <string.h> since it's not currently in use.

  • All of your operations rely on command line arguments, yet the program is still allowed to continue even if there aren't any (excluding the filename). You can fix this by terminating the program right away if argc is too low.

    if (argc == 1)
    {
        // print this to the standard error stream
        std::cerr << "Additional arguments are required!";
        // same as return 1, also used for failure
        return EXIT_FAILURE;
    }
    

    With this in place, you should also redo the other argc checks. Add this given check first, then have the others. I'd also consider replacing else with an else if that includes a specific number of command line arguments. With the else there, that code block will be executed with any other value not already given in an if.

  • Consider changing the parameter names of the functions for clarity. Even if you're trying to pass the command line arguments to them, the functions themselves aren't very self-documenting.

    You could also assign the various argv[] values to separate variables in main() and passing them to the other functions. This again helps with self-documentation.

  • There's no need to declare and then initialize:

    double operand;
    operand = strtod(argv[2], NULL );
    

    If you just need to initialize, then do that only:

    double operand = strtod(argv[2], NULL );
    
  • The contents of mean():

    double summation = sum(argc, argv);
    return summation/((double) argc-2);
    

    can just be a single line:

    return sum(argc, argv) / ((double) argc-2);
    

    The sum() function still reveals the intent, so it can be put together.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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