Read & write text file issues

[Airynoob72]

I'm afraid I'm just going round and round in circles and not actually making any progress.

I need the program to open both an input and output file from the command line. It will then read the input file on each line and get the values allocated to location 41-50 (should be digits, will throw an error and skip that line if not digits- also has to delete the blank spaces).
Then the program should be able to add only the line values which are all digits, output the sum and average to the output file and show on screen.

Here's what I've got currently. It's giving me a warning currently
":91:1: warning: control reaches end of non-void function", but it compiles. When I go to run it, it shows a pop up saying it's stopped working and exits the program. I'm not sure where to actually start to fix it and get myself on track.

#include <iostream>
#include <algorithm> 
#include <string>
#include <fstream>
#include <cctype>
#include <sstream>
#include <iomanip>


using namespace std;


double string2double(string s);
bool checkDigits(string check);
void addNumbers(int tempNum, int sum, int count);


int main(int argc, char* argv[]){

int sum = 0;
int count = 0;
double average = 0;
string tempStr, numStr;
int numInt;
fstream infile;
fstream outfile;

// Open input file using command prompt.
//openFile();

// If name given in the command prompt or not enough arguments, throw error exception
    infile.open(argv[1]);
    outfile.open(argv[1]);
    infile.seekp(0, ios::beg); 
    
// Reads input file line by line, extracts relevant payment amount (should be in location 41-50).
while (infile){
    getline(infile, tempStr);
    numStr = tempStr.substr(40,10);
    numStr.erase(numStr.find_last_not_of(" \n\r\t")+1);

// Check to see if payment is a digit or not
    while (!checkDigits(numStr)){
    cout << "Number contains non digits: " << numStr << "\n Current Line will be Skipped!\n\n";
    break;
    } 
    
   
    numInt = string2double(numStr);
    addNumbers(numInt, sum, count);
    
    
}

// If payment is not correct (not all digits), skip the line and display error message.

//Calculate average payment
    average = sum/count;
    
// Open the output file and write the total and average payment values. 
//Output current date & time, total & average payment to screen.

outfile << "Total Payment: $" << sum << endl;
outfile  << "Average Payment: $" << average << endl;



    return 0;
}

// Calculate total of payments 
void addNumbers(int tempNum, int sum, int count){
        
        sum += tempNum;
        ++count;
    
}    

//Check if any non-digit numbers in string
bool checkDigits(string check){
  for (int unsigned i = 0; i < check.length();){
       if (isdigit(check[i])){
        i++;  
       } else {
        return false; 
       }
   return true;
   } 
}

// Convert string to double
double string2double(string s){ 
    istringstream instr(s); 
    double n; 
    instr >> n; 
    return n; 
} 

I know it's quite messy still, I'm planning on tidying it up once I figure out wtf I'm doing wrong :(

Thanks so much

[gunnerfunner]

post a few lines of your sample file so that the program can be tested

[ne555]

learn to indent.

> warning: control reaches end of non-void function
don't remove the context. `In function ‘bool checkDigits(std::__cxx11::string)’:'
now look at that function properly indented

// Check if any non-digit numbers in string
bool checkDigits(string check) {
	for(int unsigned i = 0; i < check.length();) {
		if(isdigit(check[i])) {
			i++;
		} else {
			return false;
		}
		return true;
	}
}

note how `return true' is inside the loop, so you only will end up checking the first character.
About the warning, if the string is empty, ¿what would be the return value?
Also, your logic seems quite obfuscated

// Check if any non-digit numbers characters in string
bool checkDigits(string check) {
	for(int unsigned K = 0; K < check.length(); ++K)
		if(not isdigit(check[K]))
			return false;
	return true;
}

infile.open(argv[1]);
outfile.open(argv[1]);

¿are you sure that you want to overwrite the input file?


> it shows a pop up saying it's stopped working and exits the program. I'm not
> sure where to actually start to fix it and get myself on track.
run through a debugger, it will point you to the line where it crashes.
valgrind may help to detect invalid access of your strings.

[jlb]

It looks like you have a function with a return path that bypasses your return statements. Look at this snippet:

//Check if any non-digit numbers in string
bool checkDigits(string check){
  for (int unsigned i = 0; i < check.length();){
       if (isdigit(check[i])){
        i++;  
       } else {
        return false; 
       }
   return true;
   } 
}

Now let's look at the same snippet with a more consistent indent style:

//Check if any non-digit numbers in string
bool checkDigits(string check){
    for (int unsigned i = 0; i < check.length();){
        if (isdigit(check[i])){
            i++;  
        } else {
            return false; 
        }
        return true;
    } 
}

Notice how there in no return statement in the condition where the for() loop doesn't execute. This is one of the hazards of using multiple return statements in functions, every path must have a return statement. You really should strive to have only one return path from your functions. This can easily be accomplished with this function by a few alterations to your logic:

//Check if any non-digit numbers in string
bool checkDigits(string check){
    bool return_value = true;
    for (int unsigned i = 0; i < check.length(); ++i){
        if (!isdigit(check[i])){
            return_value = false;
        }
    }
    return return_value;
}

[ne555]

> You really should strive to have only one return path from your functions
no.

[benhart]

It's pretty hard to understand exactly what is going in there but those kind of errors occur a lot and you need to know how to detect them. There are a lot of information on the web you can use.
There are also some program to help you do that, such as checkmarx and others.
Good luck with it, waiting for more information.
Ben.

[Airynoob72]

Thanks so much for all your advice. I'm going to throw this code out the window and just start again with a clean slate and your suggestions above.