HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Adding two numbers as strings from a textfile

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numbersaddingtwotextfilefromstrings

Problem

I just implemented code to add two ints together as strings and I'm sure my method is inferior. I've looked up other methods too but I'd appreciate criticisms specific to my code. I really want to make sure I don't develop bad habits in C++ and I think leveraging the knowledge of brighter individuals than myself on this site will help with that :).
Am I relying too much on iterators btw?

Anyhow, the code is below and be as harsh as you want. Thanks!

//This program intends to read very large integers from a text file
//the format is two integers per line. It will then add them together and output
//the sums one each line

//first, succesfully read from the text file and store into two vectors
//each digit being an element
#include
#include
#include
#include
using namespace std;

int main()
{

    string line,s1,s2;
    vector vec1, vec2; //string 1 and string 2 store each of the 2 numbers per line as individual digits
    vector final;      //create vector of strings that holds the sum of each pair of strings per line
    ifstream myfile("bignum.txt"); //this opens the file bignum.txt
    if (myfile.is_open())
    {
       int  count=0; //count lines
       while (!myfile.eof())
       {
           getline(myfile,line);
           bool flag=true; //bool to see if integer 2 should be stored now
           for (int i=0; i::iterator i1=final.begin();
        int count3=0;
        for (; i1!=final.end(); i1++)
        {
            count3++;
            cout<<"The sum of the numbers on line "<<count3<<" is "<<*i1<<"\n";
        }

    }
    else cout <<"file could not be opened";
    return 0;
}

Solution

Some basic C++ advice:

-
Don't use using namespace std;. It leads to namespace pollution: it you use another library, you may import names in the global namespace that may clash with those in the std namespace. Anyway, adding std:: in front of the standard library components does not hinder readability.

-
return 0; at the end of main is useless. If the program reaches the end of main without having encountered any return statement, it will automagically return 0.

-
You compare signed and unsigned integer values in this line:

for (int i=0; i<line.length(); i++)


That means that you didn't turn on the right compiler flags (-Wall, -pedantic and -Wextra flags that you should almost always use) or that you ignored the warnings. Try never to ignore warnings, they help to write good code. To fix this warning, simply turn int i=0 into std::size_t i=0. Ideally, you should even write typename std::string::size_type i=0 but people generally agree that it is too long, and the main standard library implementations use std::size_t anyway for the length of a string.

The rationale for this warning is that in some cases, the comparison will happen, but small negative integers may be compared to big unsigned integers. On theo ther hand, if you compare unsigned integers with different sizes, you will never have such a problem.

-
It's really no more than a tidbit, but post-incrementation (++i) may be faster than pre-incrementation (i++). We all know that premature optimization is the root of all evils, but this one is simple enough to write as you go. In other words, you should remember this useful guideline:


Use ++i if you don't have a specific reason to use i++.

-
The variable count is never used, you can remove it. You compiler should have told you that it was unused.

-
Try to organize your headers in alphabetical order:

#include
#include
#include
#include


It will help you when you want to check whether a header is already included or not.

-
If you want to check whether an ifstream was successfully openen, don't use is_open, use operator!:

ifstream myfile("bignum.txt"); //this opens the file bignum.txt
if (not myfile)
{
    // An error occured
    // handle the error
}
// else use myfile


-
Also, this it not the idiomatic way to read a file line by line:

while (!myfile.eof())
{
    getline(myfile,line);
    // ...
}


Here is how you should do it, std::getline has been designed to be used in such a loop:

while (getline(myfile, line))
{
    // ...
}


Actually, using eof can read past end of file which makes it unsafe to read a file line by line until the end of the file is reached.

Code Snippets

for (int i=0; i<line.length(); i++)
#include<fstream>
#include<iotream>
#include<string>
#include<vector>
ifstream myfile("bignum.txt"); //this opens the file bignum.txt
if (not myfile)
{
    // An error occured
    // handle the error
}
// else use myfile
while (!myfile.eof())
{
    getline(myfile,line);
    // ...
}
while (getline(myfile, line))
{
    // ...
}

Context

StackExchange Code Review Q#49817, answer score: 5

Revisions (0)

No revisions yet.