vector<string>.clear() now working -- please help

[Gaurav]

Hello

I have a program that basically inverts the contents of files except
first line.

It compiles fine but gives me core dump on running. If i comment
temp.clear() it runs fine, but i need it to clear the temp vector for
each file.

********************* code *******************
#include <fstream>
#include <string>
#include <vector>
#include <iostream>

using namespace std;

// This program just inverts the tickers.csv files execpt first line
int main(){

string ticker, line;
string input,output;
ifstream Tickers( "tickers.txt", ios::in);
ifstream Input_File;
ofstream Output_File;
vector <string> temp;

while(Tickers>>ticker){
input = "tempdata/" + ticker + ".csv";
output = "tempdata/" + ticker + "1.csv";
Input_File.open( input.c_str(), ios::in);
Output_File.open ( output.c_str(), ios::app);
while ( Input_File >> line ){
temp.push_back(line);
}
Output_File << temp[0] << endl;
for ( unsigned i = ( temp.size() - 1 ); i > 0; i--)
Output_File << temp[i] << endl;
Input_File.close();
Output_File.close();
temp.clear();
}
return 0;
}
***************************************

tickers.txt contains part of file name. tempdata is a subdirectory. i
have the files to be inverted present in tempdata.

thank you in advance.

[red floyd]

Gaurav wrote:

Hello

I have a program that basically inverts the contents of files except
first line.

It compiles fine but gives me core dump on running. If i comment
temp.clear() it runs fine, but i need it to clear the temp vector for
each file.

********************* code *******************
#include <fstream>
#include <string>
#include <vector>
#include <iostream> #include <algorithm>
using namespace std;

// This program just inverts the tickers.csv files execpt first line
int main(){

string ticker, line;
string input,output;
ifstream Tickers( "tickers.txt", ios::in);
ifstream Input_File;
ofstream Output_File;
vector <string> temp;

while(Tickers>>ticker){
input = "tempdata/" + ticker + ".csv";
output = "tempdata/" + ticker + "1.csv";
Input_File.open( input.c_str(), ios::in);
Output_File.open ( output.c_str(), ios::app); while ( Input_File >> line ){ while ( std::getline(Input_File, line) )
temp.push_back(line);
}
Output_File << temp[0] << endl; for ( unsigned i = ( temp.size() - 1 ); i > 0; i--)
Output_File << temp[i] << endl; std::copy(temp.rbegin(), temp.rend() - 1, std::ostream_iterator(Output_File));
Input_File.close();
Output_File.close();
temp.clear();
}
return 0;
}
***************************************

tickers.txt contains part of file name. tempdata is a subdirectory. i
have the files to be inverted present in tempdata.

thank you in advance.

Other than the getline (in case you have blanks in a line), it looks OK to me...
Of course, I'm not a guru...

[tom_usenet]

On 8 Oct 2003 10:27:40 -0700, ba********@hotmail.com (Gaurav) wrote:

Hello

I have a program that basically inverts the contents of files except
first line.

It compiles fine but gives me core dump on running. If i comment
temp.clear() it runs fine, but i need it to clear the temp vector for
each file.

********************* code *******************
#include <fstream>
#include <string>
#include <vector>
#include <iostream>

using namespace std;

// This program just inverts the tickers.csv files execpt first line
int main(){

string ticker, line;
string input,output;
ifstream Tickers( "tickers.txt", ios::in);
ifstream Input_File;
ofstream Output_File;
vector <string> temp;
You should get out of the habit of declaring your variables at the top
of the functions. There is not need in C++, and it makes code much
less readable. In addition, it means you can't initialize them at
construction.

while(Tickers>>ticker){
input = "tempdata/" + ticker + ".csv";
output = "tempdata/" + ticker + "1.csv";
Input_File.open( input.c_str(), ios::in);
Output_File.open ( output.c_str(), ios::app);
while ( Input_File >> line ){
temp.push_back(line);
}
Shouldn't that be:
while (getline(Input_File, line)){
temp.push_back(line);
}
Output_File << temp[0] << endl;
for ( unsigned i = ( temp.size() - 1 ); i > 0; i--)
Output_File << temp[i] << endl;

Better would be (using the <algorithm> and <iterator> headers):

std::copy(temp.rbegin(), temp.rend(),
std::ostream_iterator<std::string>(Output_File, "\n"));

This also has the benefit of not crashing when temp.size() is 0, which
I suspect is your problem.

Here's the complete program. Notice that it is quite a lot shorter
thanks to declaring the variables only once they are needed:

#include <fstream>
#include <string>
#include <vector>
#include <algorithm>
#include <iterator>
#include <iostream>

using namespace std;

// This program just inverts the tickers.csv files execpt first line
int main(){

ifstream Tickers( "tickers.txt", ios::in);
string ticker;

while(Tickers>>ticker){
string input = "tempdata/" + ticker + ".csv";
string output = "tempdata/" + ticker + "1.csv";
ifstream Input_File( input.c_str(), ios::in);
ofstream Output_File( output.c_str(), ios::app);
vector <string> temp;
while ( Input_File >> line ){
temp.push_back(line);
}
std::copy(
temp.rbegin(),
temp.rend(),
std::ostream_iterator<string>(Output_File, "\n")
);
}
return 0;
}

Tom

[jeffc]

"tom_usenet" <to********@hotmail.com> wrote in message
news:4c********************************@4ax.com...

int main(){

string ticker, line;
string input,output;
ifstream Tickers( "tickers.txt", ios::in);
ifstream Input_File;
ofstream Output_File;
vector <string> temp;

You should get out of the habit of declaring your variables at the top
of the functions. There is not need in C++, and it makes code much
less readable. In addition, it means you can't initialize them at
construction.

Say what?

[tom_usenet]

On Wed, 8 Oct 2003 15:54:30 -0400, "jeffc" <no****@nowhere.com> wrote:

"tom_usenet" <to********@hotmail.com> wrote in message
news:4c********************************@4ax.com.. .

>int main(){
>
> string ticker, line;
> string input,output;
> ifstream Tickers( "tickers.txt", ios::in);
> ifstream Input_File;
> ofstream Output_File;
> vector <string> temp;

You should get out of the habit of declaring your variables at the top
of the functions. There is not need in C++, and it makes code much
less readable. In addition, it means you can't initialize them at
construction.

Say what?

The OPs code was C style, with all the declarations at the top of the
function. This is bad C++ style, for a number of reasons.

Tom

[jeffc]

"tom_usenet" <to********@hotmail.com> wrote in message
news:a4********************************@4ax.com...

On Wed, 8 Oct 2003 15:54:30 -0400, "jeffc" <no****@nowhere.com> wrote:

"tom_usenet" <to********@hotmail.com> wrote in message
news:4c********************************@4ax.com.. .

>int main(){
>
> string ticker, line;
> string input,output;
> ifstream Tickers( "tickers.txt", ios::in);
> ifstream Input_File;
> ofstream Output_File;
> vector <string> temp;

You should get out of the habit of declaring your variables at the top
of the functions. There is not need in C++, and it makes code much
less readable. In addition, it means you can't initialize them at
construction.

Say what?

The OPs code was C style, with all the declarations at the top of the
function. This is bad C++ style, for a number of reasons.

Yes, but because you can't initialize them at construction isn't one of
them. In fact, I've sometimes used the instantiation of a dummy object as a
trick to get an early entry point into some code when a DLL or something is
loaded (the constructor for the object gives you the entry point, from where
you can write whatever code you want.)

[Jonathan Mcdougall]

> > >> You should get out of the habit of declaring your variables at the
top

> of the functions. There is not need in C++, and it makes code much
> less readable. In addition, it means you can't initialize them at
> construction.

Say what?
The OPs code was C style, with all the declarations at the top of the
function. This is bad C++ style, for a number of reasons.

Yes, but because you can't initialize them at construction isn't one of
them.

It is the major one.

std::string s("hello");

is more "efficient" than

std::string s;
s = "hello";

since that constructor is more "efficient" than creating an empty string
and then use the assigment operator.
In fact, I've sometimes used the instantiation of a dummy object as a
trick to get an early entry point into some code when a DLL or something is loaded (the constructor for the object gives you the entry point, from where you can write whatever code you want.)

So what?
Jonathan

[Gavin Deane]

"Jonathan Mcdougall" <jo***************@DELyahoo.ca> wrote in message news:<IG**********************@weber.videotron.net >...

>> You should get out of the habit of declaring your variables at the top >> of the functions. There is not need in C++, and it makes code much
>> less readable. In addition, it means you can't initialize them at
>> construction.
>
>Say what?

The OPs code was C style, with all the declarations at the top of the
function. This is bad C++ style, for a number of reasons.

Yes, but because you can't initialize them at construction isn't one of
them.

It is the major one.

std::string s("hello");

is more "efficient" than

std::string s;
s = "hello";

since that constructor is more "efficient" than creating an empty string
and then use the assigment operator.

Also, you might not know at the top of the function what the initial
value or constructor parameters are going to be.

GJD

[Thomas Matthews]

Jonathan Mcdougall wrote:

>You should get out of the habit of declaring your variables at the
top
of the functions. There is not need in C++, and it makes code much
>less readable. In addition, it means you can't initialize them at
>construction.

Say what?

The OPs code was C style, with all the declarations at the top of the
function. This is bad C++ style, for a number of reasons.

Yes, but because you can't initialize them at construction isn't one of
them.

It is the major one.

std::string s("hello");

is more "efficient" than

std::string s;
s = "hello";

since that constructor is more "efficient" than creating an empty string
and then use the assigment operator.

Not necessarily. Efficiency is up to the compiler. A good compiler
could recognize the assignment following a declaration and combine
the two as an optimization.

[1] std::string s("hello")
The above executes the constructor with the value "hello".

[2] std::string s;
s = "hello"
The above executes the default constructor then calls the assignment
operator. This may be converted to [1] above by the compiler by
an optimization.

By the way, these kinds of optimizations are actually trivial and
waste more development time then they gain in performanace. The
current school of thought is to get the program working correctly
and finished before worrying about optimizations. Who knows,
perhaps these declarations may be eliminated through a design
or requirements optimization.

--
Thomas Matthews

C++ newsgroup welcome message:
http://www.slack.net/~shiva/welcome.txt
C++ Faq: http://www.parashift.com/c++-faq-lite
C Faq: http://www.eskimo.com/~scs/c-faq/top.html
alt.comp.lang.learn.c-c++ faq:
http://www.raos.demon.uk/acllc-c++/faq.html
Other sites:
http://www.josuttis.com -- C++ STL Library book

[lilburne]

Jonathan Mcdougall wrote:

>You should get out of the habit of declaring your variables at the
top
of the functions. There is not need in C++, and it makes code much
>less readable. In addition, it means you can't initialize them at
>construction.

Say what?

The OPs code was C style, with all the declarations at the top of the
function. This is bad C++ style, for a number of reasons.

Yes, but because you can't initialize them at construction isn't one of
them.

It is the major one.

std::string s("hello");

is more "efficient" than

std::string s;
s = "hello";

since that constructor is more "efficient" than creating an empty string
and then use the assigment operator.

Perhaps. But it is dependent on the implementation of the
class. Putting the declarations, as the was done in the
rewrite, inside a loop could be terribly inefficient. Say,
for example, the assignment operator reuses the old
character buffer if the new string is smaller than that
which is being replaced? In cases where memory is reallocate
each time through the loop you can lose painfully. I've seen
a 17% speed improvement by declaring a class variable that
allocated memory outside of a loop.

You never know what is, or is not efficient unless you
profile the code. Assuming that a certain construct is more
efficient will cause you no end of surprises. Get it right
first and then if you need to tune performance profile.

[Jonathan Mcdougall]

> >>>>>You should get out of the habit of declaring your variables at the

top

>>of the functions. There is not need in C++, and it makes code much
>>less readable. In addition, it means you can't initialize them at
>>construction.
>
>Say what?

The OPs code was C style, with all the declarations at the top of the
function. This is bad C++ style, for a number of reasons.

Yes, but because you can't initialize them at construction isn't one of
them.

It is the major one.

Perhaps. But it is dependent on the implementation of the
class. Putting the declarations, as the was done in the
rewrite, inside a loop could be terribly inefficient.

<snip>

My point was not that it is always bad to use default
ctors. There are certain circumstances where you just cannot
provide a value or the default contructor does some trivial
operations compared to another one.

And yes, I know that the optimizations are implementation defined and
that 'efficiency is up to the compiler', which is why the two
occurences of 'efficient' in my post were between double-quotes.

"Efficiency" was to be taken in the sense "correct" or "better", not
"faster" or "cheaper".
Jonathan

[Gandalf]

> By the way, these kinds of optimizations are actually trivial and

waste more development time then they gain in performanace. The
current school of thought is to get the program working correctly
and finished before worrying about optimizations. Who knows,
perhaps these declarations may be eliminated through a design
or requirements optimization.

Well, I say, change the current school then.
You should optimize with pen and paper before you write a single line of
code. Creating "something that works" is not a substitute for making a good
design.

[Karl Heinz Buchegger]

Gandalf wrote:

By the way, these kinds of optimizations are actually trivial and
waste more development time then they gain in performanace. The
current school of thought is to get the program working correctly
and finished before worrying about optimizations. Who knows,
perhaps these declarations may be eliminated through a design
or requirements optimization.

Well, I say, change the current school then.
You should optimize with pen and paper before you write a single line of
code. Creating "something that works" is not a substitute for making a good
design.

Nobody talked about 'design efficiency' in this thread.
But we talked about optimization by fiddeling at the bit level, which
is usually done better by the compiler.

Of course you are right: Using a quick sort instead of a bubble
sort is some sort of 'optimization' one should make. But there
is no point in 'optimizing' bubble sort by making clever C++ hacks.

--
Karl Heinz Buchegger
kb******@gascad.at