while (!fin.eof())
Is one of the common sins of C++. It does not do what you want it to do. The oversimplification is you cannot know if you've reached the end of the file unless you've tried to read past it. This means you get an extra read that fails and undefined results if you don't test that read for success.
Testing reading for success is really easy and covers nearly every failure case you are likely to run up against in a school assignment, including EOF.
while (fin >> buffer)
{
// do stuff
}
That means most of your read logic compresses down to:
if (!fin)
{
cout << "can't read the file!!" << endl;
return -1;
}
while (fin >> buffer)
{
// do stuff
}
Before we get going on what the assignment requires, The proper way to do what you wish to do is
int tempnumber;
std::vector<int> numbers;
while (fin >> tempnumber) // read directly into a number. No further parsing needed.
{
numbers.push_back(tempnumber);
}
if (fin.eof()) // now is when you check eof.
// If we didn't get to the end of the file,
// there is something wrong with the file
{
std::cout << "Before sort:" << std::endl;
for (int number: numbers)
{
std::cout << " " << number << std::endl;
}
// And because I know why you are doing this, you finish off with:
std::sort(numbers.begin(), numbers.end());
std::cout << "After sort:" << std::endl;
for (int number: numbers)
{
std::cout << " " << number << std::endl;
}
}
else // stopped somewhere other than the end of the file. Bad file
{
std::cerr << "improperly formatted file" << std::endl;
return 0;
}
Sadly OP has an instructor who believes that to learn to shoot a rifle you should first learn to build the rifle, so OP cannot take advantage of C++'s higher level features. I'm more a teach logic, then teach the low-level language fundamentals kind of guy, so of course I think this teaching style is sub-optimal.
Also note the use of explicit namespacing. using namespace std; carries risks that, in my opinion, outweigh the difficulty of typing a few extra characters.
But how to do this with a dynamically sized array without high-level constructs? OP read the file twice: once for size and the second time to get content. Slow, file IO is awesomely slow compared to just about anything else you do on a computer, but works. To not read the file twice, a simple singly-linked list could work, but OP has stated (in their previous question) that they ultimately need an array to sort the list.
Without that unmentioned requirement, this solution resolves down to don't use an array, don't convert to a number, and
while (fin >> buffer)
{
cout << buffer<< endl;
}
Done. There are other fun tricks that dump fin directly into cout, but in essence, it's doing that.
This can be solved by defining a class that does the dynamic array heavy lifting, but why? std::vector fills that market niche, and why rob OP's instructor of what is likely to be the next assignment?
So let's do this by manually resizing an array. What we are going to do is brute force and idiocy. Every time the array is about to over flow, we make a new array that's twice the size of the old one, copy the old into the new, release the old and point at the new.
#include <iostream>
#include <fstream>
#include <cstdlib>
#include <cstring>
using namespace std;
int main(int argc, char* argv[])
{
if (argc > 1)
{
char buffer[100]; // no need for pointer. allocate a big buffer that is
// unlikely to overflow
int * numbers = new int[10]; // make a dynamic array. 10 seems like a
// good starting size
int * is called a raw pointer. It is unprotected. It must be manually managed. If anything goes wrong, it is often lost and what it points at becomes effectively unrecoverable. Do not use raw pointers in the real world without a really good reason that you can honestly justify and explain to other people. And if one of those other people says, "But you can do this instead." and they are right, do that instead.
Before considering a raw pointer examine using std::unique_ptr and [std::shared_ptr][5] in that order. There are a whole raft of better ways to do this, but odds are the instructor will refuse to allow them. Know that they exist and use them when you can. They will save you a lot of grief.
size_t capacity = 10; // number of items that can go into numbers
When making index variables, favour size_t. It is unsigned (no values < 0) and guaranteed to be able to store the size in bytes of the biggest object that can be constructed. In other words, if it isn't big enough your program cannot work.
size_t size = 0; // number of items currently in numbers
ifstream fin(argv[1]); //open the file
while (fin >> buffer) // will read up to next whitespace in file.
This can read past the end of the buffer. Be warned. This is why we made buffer big.
{
int temp = atoi(buffer);
atoi is a crude and fault-prone left over from the stone ages of C. I'm not slagging C here. A sane C programmer uses it with the same amount of caution because it is a very old and very stupid function. Only use it under protest or in extremely controlled circumstances.
Prefer strtol and its family for converting char arrays and std::stoi and family for converting std::strings because they can catch errors for you. Prefer using std::strings to char arrays.
if (capacity == size) // need to resize numbers
{ // make a bigger array, copy into bigger array, replace smaller array
int * temp = new int[capacity * 2]; // make a bigger array
memcpy(temp, numbers, capacity * sizeof(numbers[0]));
Like atoi, memcpy is crude, but works. If vector is out for this assignment, std::copy is probably out as well. memcpy is fine for simple values like int and double, but favour std::copy for structures and classes. memcpy mindlessly copies exactly what is in an object or structure, they often have other things going on behind the scenes, like pointers, that need special handling when copied.
delete numbers; // free the array currently used by numbers
numbers = temp; // point at new, bigger array
capacity *= 2; // update the capacity to the new size
}
numbers[size++] = temp;
}
std::cout << "Before sort:" << std::endl;
for (size_t index = 0; index < size; index++)
{
std::cout << " " << numbers[index] << std::endl;
}
fin.close();
// call to sorting function goes here.
}
return 0;
}
And in one cut-n-pastable block:
#include <iostream>
#include <fstream>
#include <cstdlib>
#include <cstring>
using namespace std;
int main(int argc, char* argv[])
{
if (argc > 1)
{
char buffer[100]; // no need for pointer. allocate a big buffer that is
// unlikely to overflow
int * numbers = new int[10]; // make a dynamic array. 10 seems like a
// good starting size
size_t capacity = 10; // number of items that can go into numbers
size_t size = 0; // number of items currently in numbers
ifstream fin(argv[1]); //open the file
while (fin >> buffer) // will read up to next whitespace in file.
{
int temp = atoi(buffer);
if (capacity == size) // need to resize numbers
{ // make a bigger array, copy into bigger array, replace smaller array
int * temp = new int[capacity * 2]; // make a bigger array
memcpy(temp, numbers, capacity * sizeof(numbers[0]));
delete numbers; // free the array currently used by numbers
numbers = temp; // point at new, bigger array
capacity *= 2; // update the capacity to the new size
}
numbers[size++] = temp;
}
std::cout << "Before sort:" << std::endl;
for (size_t index = 0; index < size; index++)
{
std::cout << " " << numbers[index] << std::endl;
}
fin.close();
// call to sorting function goes here.
}
return 0;
}
Now that we have the logic down, there are a whole slew of improvements. First is get all of that code out of main and into its own read function. That way main is nice and clean with two well defined subtasks each in their own function: read and sort.
Why? It's much easier for the human mind to comprehend small stuff than big. It's also much easier to keep the human mind focused if there is only one problem at a time. read function is shorter than one big read and sort function. read and sort means two things are going on at once. And the big fun comes from when read interferes with the functioning of sort. Which do you debug? both of them. Keep separate jobs separate unless there are significant, measurable, and utterly necessary gains from co-mingling.
If you plow everything into main, you get one massive, confused mess. Reading the code becomes harder. That means debugging becomes harder. It takes longer to do assignments. On the more immediate front, marking becomes harder. Grade becomes lower.
In real life as a programmer, add on mockery by one's peers, failed code reviews, and ultimately being released from your job because your personal productivity sucks and cleaning up your code drags down the performance of your whole department.
