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.tx t", 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.ope n ( 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.clos e();
Output_File.clo se();
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]

Re: vector&lt;strin g&gt;.clear() now working -- please help

Gaurav wrote:
[color=blue]
> 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>[/color]
#include <algorithm>[color=blue]
>
> 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.tx t", 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.ope n ( output.c_str(), ios::app);[/color]
[color=blue]
> while ( Input_File >> line ){[/color]
while ( std::getline(In put_File, line) )
[color=blue]
> temp.push_back( line);
> }
> Output_File << temp[0] << endl;[/color]
[color=blue]
> for ( unsigned i = ( temp.size() - 1 ); i > 0; i--)
> Output_File << temp[i] << endl;[/color]
std::copy(temp. rbegin(), temp.rend() - 1, std::ostream_it erator(Output_F ile));
[color=blue]
> Input_File.clos e();
> Output_File.clo se();
> 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.[/color]

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]

Re: vector&lt;strin g&gt;.clear() now working -- please help

On 8 Oct 2003 10:27:40 -0700, bansal2425@hotm ail.com (Gaurav) wrote:
[color=blue]
>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.tx t", ios::in);
> ifstream Input_File;
> ofstream Output_File;
> vector <string> temp;[/color]

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.
[color=blue]
>
> while(Tickers>> ticker){
> input = "tempdata/" + ticker + ".csv";
> output = "tempdata/" + ticker + "1.csv";
> Input_File.open ( input.c_str(), ios::in);
> Output_File.ope n ( output.c_str(), ios::app);
> while ( Input_File >> line ){
> temp.push_back( line);
> }[/color]

Shouldn't that be:
while (getline(Input_ File, line)){
temp.push_back( line);
}


[color=blue]
> Output_File << temp[0] << endl;
> for ( unsigned i = ( temp.size() - 1 ); i > 0; i--)
> Output_File << temp[i] << endl;[/color]

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

std::copy(temp. rbegin(), temp.rend(),
std::ostream_it erator<std::str ing>(Output_Fil e, "\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.tx t", 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_it erator<string>( Output_File, "\n")
);
}
return 0;
}

Tom

[jeffc]

Re: vector&lt;strin g&gt;.clear() now working -- please help


"tom_usenet " <tom_usenet@hot mail.com> wrote in message
news:4cj8ov4mq4 avlkl9knkn2lkm4 6jkv1oi18@4ax.c om...[color=blue][color=green]
> >int main(){
> >
> > string ticker, line;
> > string input,output;
> > ifstream Tickers( "tickers.tx t", ios::in);
> > ifstream Input_File;
> > ofstream Output_File;
> > vector <string> temp;[/color]
>
> 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.[/color]

Say what?

[tom_usenet]

Re: vector&lt;strin g&gt;.clear() now working -- please help

On Wed, 8 Oct 2003 15:54:30 -0400, "jeffc" <nobody@nowhere .com> wrote:
[color=blue]
>
>"tom_usenet " <tom_usenet@hot mail.com> wrote in message
>news:4cj8ov4mq 4avlkl9knkn2lkm 46jkv1oi18@4ax. com...[color=green][color=darkred]
>> >int main(){
>> >
>> > string ticker, line;
>> > string input,output;
>> > ifstream Tickers( "tickers.tx t", ios::in);
>> > ifstream Input_File;
>> > ofstream Output_File;
>> > vector <string> temp;[/color]
>>
>> 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.[/color]
>
>Say what?[/color]

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]

Re: vector&lt;strin g&gt;.clear() now working -- please help


"tom_usenet " <tom_usenet@hot mail.com> wrote in message
news:a4s8ovs7nu 4i57ri98mu33hli sd5g0hq92@4ax.c om...[color=blue]
> On Wed, 8 Oct 2003 15:54:30 -0400, "jeffc" <nobody@nowhere .com> wrote:
>[color=green]
> >
> >"tom_usenet " <tom_usenet@hot mail.com> wrote in message
> >news:4cj8ov4mq 4avlkl9knkn2lkm 46jkv1oi18@4ax. com...[color=darkred]
> >> >int main(){
> >> >
> >> > string ticker, line;
> >> > string input,output;
> >> > ifstream Tickers( "tickers.tx t", 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.[/color]
> >
> >Say what?[/color]
>
> 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.[/color]

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]

Re: vector&lt;strin g&gt;.clear() now working -- please help

> > >> You should get out of the habit of declaring your variables at the
top[color=blue][color=green][color=darkred]
> > >> 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?[/color]
> >
> > 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.[/color]
>
> Yes, but because you can't initialize them at construction isn't one of
> them.[/color]

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.
[color=blue]
>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[/color]
is[color=blue]
> loaded (the constructor for the object gives you the entry point, from[/color]
where[color=blue]
> you can write whatever code you want.)[/color]

So what?


Jonathan

[Gavin Deane]

Re: vector&lt;strin g&gt;.clear() now working -- please help

"Jonathan Mcdougall" <jonathanmcdoug all@DELyahoo.ca > wrote in message news:<IG1hb.102 110$282.1789655 @weber.videotro n.net>...[color=blue][color=green][color=darkred]
> > > >> You should get out of the habit of declaring your variables at the[/color][/color]
> top[color=green][color=darkred]
> > > >> 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.[/color]
> >
> > Yes, but because you can't initialize them at construction isn't one of
> > them.[/color]
>
> 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.[/color]

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]

Re: vector&lt;strin g&gt;.clear() now working -- please help

Jonathan Mcdougall wrote:
[color=blue][color=green][color=darkred]
>>>>>You should get out of the habit of declaring your variables at the[/color][/color]
>
> top
>[color=green][color=darkred]
>>>>>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
>>>>>constructi on.
>>>>
>>>>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.[/color]
>>
>>Yes, but because you can't initialize them at construction isn't one of
>>them.[/color]
>
>
> 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.
>[/color]

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.l earn.c-c++ faq:

http://www.raos.demon.uk/acllc-c++/faq.html

Other sites:
http://www.josuttis.com -- C++ STL Library book

[lilburne]

Re: vector&lt;strin g&gt;.clear() now working -- please help

Jonathan Mcdougall wrote:[color=blue][color=green][color=darkred]
>>>>>You should get out of the habit of declaring your variables at the[/color][/color]
>
> top
>[color=green][color=darkred]
>>>>>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
>>>>>constructi on.
>>>>
>>>>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.[/color]
>>
>>Yes, but because you can't initialize them at construction isn't one of
>>them.[/color]
>
>
> 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.
>[/color]

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]

Re: vector&lt;strin g&gt;.clear() now working -- please help

> >>>>>You should get out of the habit of declaring your variables at the[color=blue][color=green]
> > top[color=darkred]
> >>>>>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
> >>>>>constructi on.
> >>>>
> >>>>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.[/color]
> >
> >
> > It is the major one.
> >[/color]
> 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.[/color]

<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]

Re: vector&lt;strin g&gt;.clear() now working -- please help

> By the way, these kinds of optimizations are actually trivial and[color=blue]
> 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.[/color]
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]

Re: vector&lt;strin g&gt;.clear() now working -- please help



Gandalf wrote:[color=blue]
>[color=green]
> > 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.[/color]
> 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.[/color]

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
kbuchegg@gascad .at