map (associative array) loses values?

[jeroenvlek]

Hi there,

I've never done this before, so I don't know about any layout
possibilities. My apologies

The problem is this:

I've written a function:

map<const char*, int> *SearchText::countWords()
{
map<const char*, int> *table = new map<const char*, int>;

(*table)["aap"] = 1;
(*table)["noot"] = 2;

cout << (*table)["aap"] << endl;
cout << (*table)["noot"] << endl;

return table;
}

Which I want to use like this:

try {
SearchText *text = new SearchText("test.txt");
map<const char*, int> *table = text->countWords();
cout << (*table)["aap"] << endl;
cout << (*table)["noot"] << endl;
}
catch(int ex) {
cout << "Could not open file." << endl;
}


However, I get the following output:

1
2
0
0

Meaning that the first two output statements (in the function itself)
do their job and the second two do not.

I guess it's some sort of allocation problem, but what could I do
different? I guess I could use maybe malloc or calloc, but shouldn't
this be also possible with new?

BTW making table static in the function didn't help.

 

[Pete Becker]

Hi there,

I've never done this before, so I don't know about any layout
possibilities. My apologies

The problem is this:

I've written a function:

map<const char*, int> *SearchText::countWords()
{
map<const char*, int> *table = new map<const char*, int>;

There's no reason to create this map on the heap. Make it a local

(*table)["aap"] = 1;
(*table)["noot"] = 2;

cout << (*table)["aap"] << endl;
cout << (*table)["noot"] << endl;

return table;
}

Which I want to use like this:

try {
SearchText *text = new SearchText("test.txt");
map<const char*, int> *table = text->countWords();
cout << (*table)["aap"] << endl;
cout << (*table)["noot"] << endl;
}
catch(int ex) {
cout << "Could not open file." << endl;
}


However, I get the following output:

1
2
0
0

Meaning that the first two output statements (in the function itself)
do their job and the second two do not.

I guess it's some sort of allocation problem, but what could I do
different? I guess I could use maybe malloc or calloc, but shouldn't
this be also possible with new?

BTW making table static in the function didn't help.

 

[Pete Becker]

There's no reason to create this map on the heap. Make it a local

Ignore this. Posted too soon. :-(

 

[Kai-Uwe Bux]

Hi there,

I've never done this before, so I don't know about any layout
possibilities. My apologies

The problem is this:

I've written a function:

map<const char*, int> *SearchText::countWords()
{
map<const char*, int> *table = new map<const char*, int>;

(*table)["aap"] = 1;
(*table)["noot"] = 2;

cout << (*table)["aap"] << endl;
cout << (*table)["noot"] << endl;

return table;
}

Which I want to use like this:

try {
SearchText *text = new SearchText("test.txt");
map<const char*, int> *table = text->countWords();
cout << (*table)["aap"] << endl;
cout << (*table)["noot"] << endl;
}
catch(int ex) {
cout << "Could not open file." << endl;
}


However, I get the following output:

1
2
0
0

Meaning that the first two output statements (in the function itself)
do their job and the second two do not.

I guess it's some sort of allocation problem,

nope.

The problem is the data structure

map< char*, ... >

Note that this will (a) store pointers to char and (b) compare those
pointers (not the strings pointed to) to identify keys. Also note that two
different string literals "aap" are _not_ guaranteed to be represented by
the same char*. That is why you don't find the recorded entries.

but what could I do different?

I guess I could use maybe malloc or calloc, but shouldn't this be also
possible with new?

On that note: why do you do dynamic allocation of the map anyway? Your code
is littered with new() for no reason (and it misses the corresponding
delete statements, too).

BTW making table static in the function didn't help.

It is not expected to.


Best

Kai-Uwe Bux

 

[jeroenvlek]

Best

Kai-Uwe Bux

ah, ok. <string, int> works like a charm!

I'm using new, because I need the table somewhere else. The try/catch
is in my main, while the function resides in the SearchText class. The
delete statements are there, I just didn't post them

Ofcourse I'm just a c++ newbie (this is my first program actually), so
what would you do different?

 

[Jim Langston]

ah, ok. <string, int> works like a charm!

I'm using new, because I need the table somewhere else. The try/catch
is in my main, while the function resides in the SearchText class. The
delete statements are there, I just didn't post them

Ofcourse I'm just a c++ newbie (this is my first program actually), so
what would you do different?

Just because you need the map somewhere else doesn't mean you should new it.
The standard containers are actually quite small in and of themselves. If
you need the map somewhere else, you are passing the pointer to it, correct?
So just pass the pointer, or better yet, a reference to it. Creating the
map in the function is not the best method.

Consider this untested code:

class SearchText
{
public:
std::map<std::string, int>& countWords();
private:
std::map<std::string, int> table;
}

std::map<std::string, int>& SearchText::countWords()
{
table["aap"] = 1;
table["noot"] = 2;

return table;
}

int main()
{
try
{
SearchText text("test.txt");
std::map<std::string, int>& table = text.countWords();

std::cout << table["aap"] << "\n" << table["noot"] << std::endl;
}
catch(int ex )
{
std::cout << "Could not open file" << std::endl;
}
}

This is just trying to show that you don't have to new everything, in fact,
you shouldn't when you don't have to.

 

[BobR]

<cut>

ah, ok. <string, int> works like a charm!

I'm using new, because I need the table somewhere else. The try/catch
is in my main, while the function resides in the SearchText class. The
delete statements are there, I just didn't post them

Ofcourse I'm just a c++ newbie (this is my first program actually), so
what would you do different?

An alternative to Jim's suggestion:

// pass by non-const reference.
void SearchText::countWords( std::map<std::string, int> &map ){
map["aap"] = 1;
map["noot"] = 2;

std::cout << map["aap"] << std::endl;
std::cout << map["noot"] << std::endl;
return;
}

// int main(){
try {

std::map<std::string, int> MyMap;

SearchText text("test.txt");
text.countWords( MyMap );
std::cout << MyMap["aap"] << std::endl;
std::cout << MyMap["noot"] << std::endl;
}
catch( int ex) {
cout << "Could not open file." << endl;
}
// return 0;
// } // main()

Sorry, I didn't test that. Post back if you have trouble ( I may have missed
something. <G>).

 

[Kai-Uwe Bux]

ah, ok. <string, int> works like a charm!

I'm using new, because I need the table somewhere else. The try/catch
is in my main, while the function resides in the SearchText class. The
delete statements are there, I just didn't post them

Ofcourse I'm just a c++ newbie (this is my first program actually), so
what would you do different?

I would start with something like

map<const std::string, int> SearchText::countWords()
{
map<std::string, int> result;

result["aap"] = 1;
result["noot"] = 2;

cout << result["aap"] << endl;
cout << result["noot"] << endl;

return result;
}

and later

try {
SearchText text ("test.txt");
map<const std::string, int> table = text->countWords(); // *
cout << table["aap"] << endl;
cout << table["noot"] << endl;
}
catch(int ex) {
cout << "Could not open file." << endl;
}


That is, on the first try, I would leave it to the compiler to optimize away
the apparent copy-construction in line (*). If profiling shows that the
compiler does not eliminate the copy-constructor calls _and_ that there is
a need to improve performance, I might change the program:


void SearchText::countWords( map<std::string, int> & result ) {
result["aap"] = 1;
result["noot"] = 2;

cout << result["aap"] << endl;
cout << result["noot"] << endl;
}

....

try {
SearchText text ("test.txt");
map<const std::string, int> table;
text->countWords( table );
cout << table["aap"] << endl;
cout << table["noot"] << endl;
}
catch(int ex) {
cout << "Could not open file." << endl;
}


or I might resort to swap() tricks (faking move semantics):


map<const std::string, int> SearchText::countWords()
{
map<std::string, int> result;

result["aap"] = 1;
result["noot"] = 2;

cout << result["aap"] << endl;
cout << result["noot"] << endl;

return result;
}

....

try {
SearchText text ("test.txt");
map<const std::string, int> table;
text->countWords().swap( table );
cout << table["aap"] << endl;
cout << table["noot"] << endl;
}
catch(int ex) {
cout << "Could not open file." << endl;
}


Also: I would have the constructor of SearchText throw something more
meaningfull than an int.


Best

Kai-Uwe Bux

 

[Jim Langston]

<cut>

ah, ok. <string, int> works like a charm!

I'm using new, because I need the table somewhere else. The try/catch
is in my main, while the function resides in the SearchText class. The
delete statements are there, I just didn't post them

Ofcourse I'm just a c++ newbie (this is my first program actually), so
what would you do different?

An alternative to Jim's suggestion:

// pass by non-const reference.
void SearchText::countWords( std::map<std::string, int> &map ){
map["aap"] = 1;
map["noot"] = 2;

std::cout << map["aap"] << std::endl;
std::cout << map["noot"] << std::endl;
return;
}

// int main(){
try {

std::map<std::string, int> MyMap;

SearchText text("test.txt");
text.countWords( MyMap );
std::cout << MyMap["aap"] << std::endl;
std::cout << MyMap["noot"] << std::endl;
}
catch( int ex) {
cout << "Could not open file." << endl;
}
// return 0;
// } // main()

Sorry, I didn't test that. Post back if you have trouble ( I may have
missed
something. <G>).

There are a lot of alternatives to mine and Bob's suggestion. In fact, if I
was designing this class myself I would totally encapsulate the map inside
the class and main would only get to it via functions. Depending on what it
would be for something like (untested code)

class SearchText
{
public:
SearchText( const std::string& FileName );
int Value( const std::string& key ) const;
int CountWords();
private:
std::map<std::string, int> Data_;
}

SearchText::SearchText( const std::string& FileName )
{
// Open file and load Data_
}

int Value( const std::string& Key ) const
{
if ( Data_.find( Key ) != Data_.end() )
return Data_[Key];
else
return 0;
}

int CountWords()
{
// Return whatever it is this is trying to count
}

I am a strong believe in data encapsulation in classes, and try to always
prevent giving points or references to my internal classes data. Anything
you need the map for in mian you can encapsulate. Even encapsulate
operator[] if you wish.

int SearchText:perator[]( const std::string& Key )
{
return Value( Key );
}

 

[jeroenvlek]

An alternative to Jim's suggestion:

// pass by non-const reference.
void SearchText::countWords( std::map<std::string, int> &map ){
map["aap"] = 1;
map["noot"] = 2;

std::cout << map["aap"] << std::endl;
std::cout << map["noot"] << std::endl;
return;
}

// int main(){
try {

std::map<std::string, int> MyMap;

SearchText text("test.txt");
text.countWords( MyMap );
std::cout << MyMap["aap"] << std::endl;
std::cout << MyMap["noot"] << std::endl;
}
catch( int ex) {
cout << "Could not open file." << endl;
}
// return 0;
// } // main()

Sorry, I didn't test that. Post back if you have trouble ( I may have
missed
something. <G>).

There are a lot of alternatives to mine and Bob's suggestion. In fact, if I
was designing this class myself I would totally encapsulate the map inside
the class and main would only get to it via functions. Depending on what it
would be for something like (untested code)

class SearchText
{
public:
SearchText( const std::string& FileName );
int Value( const std::string& key ) const;
int CountWords();
private:
std::map<std::string, int> Data_;

}

SearchText::SearchText( const std::string& FileName )
{
// Open file and load Data_

}

int Value( const std::string& Key ) const
{
if ( Data_.find( Key ) != Data_.end() )
return Data_[Key];
else
return 0;

}

int CountWords()
{
// Return whatever it is this is trying to count

}

I am a strong believe in data encapsulation in classes, and try to always
prevent giving points or references to my internal classes data. Anything
you need the map for in mian you can encapsulate. Even encapsulate
operator[] if you wish.

int SearchText:perator[]( const std::string& Key )
{
return Value( Key );



}- Tekst uit oorspronkelijk bericht niet weergeven -

- Tekst uit oorspronkelijk bericht weergeven -- Tekst uit oorspronkelijk bericht niet weergeven -

- Tekst uit oorspronkelijk bericht weergeven -

Thanks a lot you guys for all the reactions!

The reason why I did not want to copy the entire map, was because I'm
going to use it for counting the words in

http://www-nlp.stanford.edu/fsnlp/statest/austen.txt

which is a 3,3 mb text file.

However, as Jim Langston already mentioned (and maybe you too Kai-Uwe
Bux, I'm going to study your examples more closely), I can encapsulate
this map into the SearchText class, since the map and the file are
related and there is no need to seperate them. Any possible new file
will need a new instance of SearchText anyway.

Thanks a lot though. I never worked via this principle of newsgroups,
but I'm going to have a look here more often!

For those of you interested: I need this for the first assignment of a
Natural Language Processing course (at University of Amsterdam) and
next assignments will build on this one, so I wanted to make this
stuff so generic as possible. Some of you now might say: Use Python!
But I really want to learn C++ (and I would have to learn Python
too...) and the only way to do this is by programming

Thanks again.

Jeroen

 

[BobR]

Jim Langston wrote in message...

class SearchText
{
public:
SearchText( const std::string& FileName );
int Value( const std::string& key ) const;
int CountWords();
private:
std::map<std::string, int> Data_;
}

Oh Lord, what's this world comeing to?!?

Do you realize you could be appointed to the U.S. Supreme Court by the bush
admin for leaving off the critical semicolon at the end of the class decl?!?

<G>

Otherwise, I agree.

#include <iostream>
int main(){
std::cout<< " Hi Jim. :-} " <<std::endl;
return 0;
}

 

[Jim Langston]

<cut>

ah, ok. <string, int> works like a charm!

I'm using new, because I need the table somewhere else. The try/catch
is in my main, while the function resides in the SearchText class. The
delete statements are there, I just didn't post them

Ofcourse I'm just a c++ newbie (this is my first program actually), so
what would you do different?

An alternative to Jim's suggestion:

// pass by non-const reference.
void SearchText::countWords( std::map<std::string, int> &map ){
map["aap"] = 1;
map["noot"] = 2;

std::cout << map["aap"] << std::endl;
std::cout << map["noot"] << std::endl;
return;
}

// int main(){
try {

std::map<std::string, int> MyMap;

SearchText text("test.txt");
text.countWords( MyMap );
std::cout << MyMap["aap"] << std::endl;
std::cout << MyMap["noot"] << std::endl;
}
catch( int ex) {
cout << "Could not open file." << endl;
}
// return 0;
// } // main()

Sorry, I didn't test that. Post back if you have trouble ( I may have
missed
something. <G>).

There are a lot of alternatives to mine and Bob's suggestion. In fact,
if I
was designing this class myself I would totally encapsulate the map
inside
the class and main would only get to it via functions. Depending on what
it
would be for something like (untested code)

class SearchText
{
public:
SearchText( const std::string& FileName );
int Value( const std::string& key ) const;
int CountWords();
private:
std::map<std::string, int> Data_;

}

SearchText::SearchText( const std::string& FileName )
{
// Open file and load Data_

}

int Value( const std::string& Key ) const
{
if ( Data_.find( Key ) != Data_.end() )
return Data_[Key];
else
return 0;

}

int CountWords()
{
// Return whatever it is this is trying to count

}

I am a strong believe in data encapsulation in classes, and try to always
prevent giving points or references to my internal classes data.
Anything
you need the map for in mian you can encapsulate. Even encapsulate
operator[] if you wish.

int SearchText:perator[]( const std::string& Key )
{
return Value( Key );



}- Tekst uit oorspronkelijk bericht niet weergeven -

- Tekst uit oorspronkelijk bericht weergeven -- Tekst uit oorspronkelijk
bericht niet weergeven -

- Tekst uit oorspronkelijk bericht weergeven -

Thanks a lot you guys for all the reactions!

The reason why I did not want to copy the entire map, was because I'm
going to use it for counting the words in

http://www-nlp.stanford.edu/fsnlp/statest/austen.txt

which is a 3,3 mb text file.

However, as Jim Langston already mentioned (and maybe you too Kai-Uwe
Bux, I'm going to study your examples more closely), I can encapsulate
this map into the SearchText class, since the map and the file are
related and there is no need to seperate them. Any possible new file
will need a new instance of SearchText anyway.

Thanks a lot though. I never worked via this principle of newsgroups,
but I'm going to have a look here more often!

For those of you interested: I need this for the first assignment of a
Natural Language Processing course (at University of Amsterdam) and
next assignments will build on this one, so I wanted to make this
stuff so generic as possible. Some of you now might say: Use Python!
But I really want to learn C++ (and I would have to learn Python
too...) and the only way to do this is by programming

I can understand not wanting to copy the map being 3.3 megabytes. Be aware,
however, that if you copy your class, the map will get copied also. So just
make sure you pass your class by reference than by value. One way to ensure
this is to disable copying of your class. The normal way to do this is to
create a copy constructor ( and assignment operator) and make them private
to the class. That way if you accidently write some code that would create
a copy of the class, it won't compile and you will realize it.

For the SearchText class that would be:

class SearchText
{
public:
// ...
private:

// Disable copy and assignment by making private.
SearchText( SearchText const&) {}
SearchText& operator=( SearchText const&) {}
}

Just empty copy constructor and assignment operator's private to the class.

 

[Jim Langston]

<cut>

ah, ok. <string, int> works like a charm!

I'm using new, because I need the table somewhere else. The try/catch
is in my main, while the function resides in the SearchText class.
The
delete statements are there, I just didn't post them

Ofcourse I'm just a c++ newbie (this is my first program actually),
so
what would you do different?

An alternative to Jim's suggestion:

// pass by non-const reference.
void SearchText::countWords( std::map<std::string, int> &map ){
map["aap"] = 1;
map["noot"] = 2;

std::cout << map["aap"] << std::endl;
std::cout << map["noot"] << std::endl;
return;
}

// int main(){
try {

std::map<std::string, int> MyMap;

SearchText text("test.txt");
text.countWords( MyMap );
std::cout << MyMap["aap"] << std::endl;
std::cout << MyMap["noot"] << std::endl;
}
catch( int ex) {
cout << "Could not open file." << endl;
}
// return 0;
// } // main()

Sorry, I didn't test that. Post back if you have trouble ( I may have
missed
something. <G>).

There are a lot of alternatives to mine and Bob's suggestion. In fact,
if I
was designing this class myself I would totally encapsulate the map
inside
the class and main would only get to it via functions. Depending on
what it
would be for something like (untested code)

class SearchText
{
public:
SearchText( const std::string& FileName );
int Value( const std::string& key ) const;
int CountWords();
private:
std::map<std::string, int> Data_;

}

SearchText::SearchText( const std::string& FileName )
{
// Open file and load Data_

}

int Value( const std::string& Key ) const
{
if ( Data_.find( Key ) != Data_.end() )
return Data_[Key];
else
return 0;

}

int CountWords()
{
// Return whatever it is this is trying to count

}

I am a strong believe in data encapsulation in classes, and try to
always
prevent giving points or references to my internal classes data.
Anything
you need the map for in mian you can encapsulate. Even encapsulate
operator[] if you wish.

int SearchText:perator[]( const std::string& Key )
{
return Value( Key );



}- Tekst uit oorspronkelijk bericht niet weergeven -

- Tekst uit oorspronkelijk bericht weergeven -- Tekst uit oorspronkelijk
bericht niet weergeven -

- Tekst uit oorspronkelijk bericht weergeven -

Thanks a lot you guys for all the reactions!

The reason why I did not want to copy the entire map, was because I'm
going to use it for counting the words in

http://www-nlp.stanford.edu/fsnlp/statest/austen.txt

which is a 3,3 mb text file.

However, as Jim Langston already mentioned (and maybe you too Kai-Uwe
Bux, I'm going to study your examples more closely), I can encapsulate
this map into the SearchText class, since the map and the file are
related and there is no need to seperate them. Any possible new file
will need a new instance of SearchText anyway.

Thanks a lot though. I never worked via this principle of newsgroups,
but I'm going to have a look here more often!

For those of you interested: I need this for the first assignment of a
Natural Language Processing course (at University of Amsterdam) and
next assignments will build on this one, so I wanted to make this
stuff so generic as possible. Some of you now might say: Use Python!
But I really want to learn C++ (and I would have to learn Python
too...) and the only way to do this is by programming

I can understand not wanting to copy the map being 3.3 megabytes. Be
aware, however, that if you copy your class, the map will get copied also.
So just make sure you pass your class by reference than by value. One way
to ensure this is to disable copying of your class. The normal way to do
this is to create a copy constructor ( and assignment operator) and make
them private to the class. That way if you accidently write some code
that would create a copy of the class, it won't compile and you will
realize it.

For the SearchText class that would be:

class SearchText
{
public:
// ...
private:

// Disable copy and assignment by making private.
SearchText( SearchText const&) {}
SearchText& operator=( SearchText const&) {}
}

Just empty copy constructor and assignment operator's private to the
class.

Just a note. I noticed in that code that in operator= I don't have a return
statement. I looked in my own code where I'm doing this same thing and also
don't have a return statement. I don't know if that's a compiler flaw not
catching it, or the standard allows it. I think, however, that
SearchText& opeator=( SearchText const& ) { return this; }
would be more correct, even though it will never be called.

 

[jeroenvlek]

<cut>
ah, ok. <string, int> works like a charm!
I'm using new, because I need the table somewhere else. The try/catch
is in my main, while the function resides in the SearchText class.
The
delete statements are there, I just didn't post them
Ofcourse I'm just a c++ newbie (this is my first program actually),
so
what would you do different?
An alternative to Jim's suggestion:
// pass by non-const reference.
void SearchText::countWords( std::map<std::string, int> &map ){
map["aap"] = 1;
map["noot"] = 2;
std::cout << map["aap"] << std::endl;
std::cout << map["noot"] << std::endl;
return;
}
// int main(){
try {
std::map<std::string, int> MyMap;
SearchText text("test.txt");
text.countWords( MyMap );
std::cout << MyMap["aap"] << std::endl;
std::cout << MyMap["noot"] << std::endl;
}
catch( int ex) {
cout << "Could not open file." << endl;
}
// return 0;
// } // main()
Sorry, I didn't test that. Post back if you have trouble ( I may have
missed
something. <G>).
There are a lot of alternatives to mine and Bob's suggestion. In fact,
if I
was designing this class myself I would totally encapsulate the map
inside
the class and main would only get to it via functions. Depending on
what it
would be for something like (untested code)
class SearchText
{
public:
SearchText( const std::string& FileName );
int Value( const std::string& key ) const;
int CountWords();
private:
std::map<std::string, int> Data_;
}
SearchText::SearchText( const std::string& FileName )
{
// Open file and load Data_
}
int Value( const std::string& Key ) const
{
if ( Data_.find( Key ) != Data_.end() )
return Data_[Key];
else
return 0;
}
int CountWords()
{
// Return whatever it is this is trying to count
}
I am a strong believe in data encapsulation in classes, and try to
always
prevent giving points or references to my internal classes data.
Anything
you need the map for in mian you can encapsulate. Even encapsulate
operator[] if you wish.
int SearchText:perator[]( const std::string& Key )
{
return Value( Key );
}- Tekst uit oorspronkelijk bericht niet weergeven -
- Tekst uit oorspronkelijk bericht weergeven -- Tekst uit oorspronkelijk
bericht niet weergeven -
- Tekst uit oorspronkelijk bericht weergeven -
Thanks a lot you guys for all the reactions!
The reason why I did not want to copy the entire map, was because I'm
going to use it for counting the words in
http://www-nlp.stanford.edu/fsnlp/statest/austen.txt
which is a 3,3 mb text file.
However, as Jim Langston already mentioned (and maybe you too Kai-Uwe
Bux, I'm going to study your examples more closely), I can encapsulate
this map into the SearchText class, since the map and the file are
related and there is no need to seperate them. Any possible new file
will need a new instance of SearchText anyway.
Thanks a lot though. I never worked via this principle of newsgroups,
but I'm going to have a look here more often!
For those of you interested: I need this for the first assignment of a
Natural Language Processing course (at University of Amsterdam) and
next assignments will build on this one, so I wanted to make this
stuff so generic as possible. Some of you now might say: Use Python!
But I really want to learn C++ (and I would have to learn Python
too...) and the only way to do this is by programming

I can understand not wanting to copy the map being 3.3 megabytes. Be
aware, however, that if you copy your class, the map will get copied also.
So just make sure you pass your class by reference than by value. One way
to ensure this is to disable copying of your class. The normal way to do
this is to create a copy constructor ( and assignment operator) and make
them private to the class. That way if you accidently write some code
that would create a copy of the class, it won't compile and you will
realize it.

For the SearchText class that would be:

class SearchText
{
public:
// ...
private:

// Disable copy and assignment by making private.
SearchText( SearchText const&) {}
SearchText& operator=( SearchText const&) {}
}

Just empty copy constructor and assignment operator's private to the
class.

Just a note. I noticed in that code that in operator= I don't have a return
statement. I looked in my own code where I'm doing this same thing and also
don't have a return statement. I don't know if that's a compiler flaw not
catching it, or the standard allows it. I think, however, that
SearchText& opeator=( SearchText const& ) { return this; }
would be more correct, even though it will never be called.

One more question though. Why are all you guys constantly typing
'std::something', instead of just typing 'using namespace std' one
time at the top of each file?

 

[Victor Bazarov]

[... excessive quoting removed ...]

One more question though. Why are all you guys constantly typing
'std::something', instead of just typing 'using namespace std' one
time at the top of each file?

http://www.parashift.com/c++-faq-lite/coding-standards.html#faq-27.5

V

 

[BobR]

Jim Langston wrote in message...

Just a note. I noticed in that code that in operator= I don't have a return
statement. I looked in my own code where I'm doing this same thing and also
don't have a return statement. I don't know if that's a compiler flaw not
catching it, or the standard allows it. I think, however, that
SearchText& opeator=( SearchText const& ) { return this; }
would be more correct, even though it will never be called.

AFAIK, you don't need to define it, just declare it.
( only define operator= if you're using it inside the class.).

class SearchText{
// Disable copy and assignment by making private.
SearchText( SearchText const&);
SearchText& operator=( SearchText const&);
// or
// void operator=( SearchText const&);
public:
// ....
};

 

[James Kanze]

Just because you need the map somewhere else doesn't mean you
should new it.

Maybe. The question isn't really so much where it is used, but
what its lifetime must be. In his case, for example, he can't
use a local variable, because the lifetime must extend beyond
the end of the function. If the lifetime corresponds to the
class in which it is created, he should make it a member
variable, but if nothing else has the correct lifetime, he's
pretty much stuck with using dynamic allocation; it's the means
C++ offers to achieve an arbitrary lifetime.

On the other hand, I don't think I've ever needed a standard
container with arbitrary lifetime; if he needs one, I suspect
that there is a design problem somewhere upstream.

The standard containers are actually quite small in and of
themselves. If you need the map somewhere else, you are
passing the pointer to it, correct? So just pass the pointer,
or better yet, a reference to it. Creating the map in the
function is not the best method.

That's probably correct. And of course, there's nothing wrong
per se with having the function return (a copy of) the map, as
long as the number of elements is reasonably small and the
function isn't called from the middle of a tight loop in a
critical section of code.

[...]

This is just trying to show that you don't have to new
everything, in fact, you shouldn't when you don't have to.

As a general rule, use dynamic allocation only when you need
explicitlly managed lifetime. (There are a few other cases, but
they probably don't concern a beginner.)

 

[James Kanze]

On Sep 11, 5:59 pm, Kai-Uwe Bux <[email protected]> wrote:

[...]
The reason why I did not want to copy the entire map, was because I'm
going to use it for counting the words in
http://www-nlp.stanford.edu/fsnlp/statest/austen.txt
which is a 3,3 mb text file.

Which isn't that relevant. What is relevant is that the file
probably contains a couple of thousand different words. Given
the performance of today's machines, I wouldn't worry too much
about copying it, provided you don't do it too often. It's
probably a good idea not to get in the habit of copying
containers all over the place, but an occasional copy, or a copy
of a container with relatively few members, isn't something to
overly worry about, if the design calls for it.

In this case, however...

[...]

I can understand not wanting to copy the map being 3.3
megabytes.

The map won't be 3.3 Megabyts. But more to the point, I can
understand not wanting to have different copies of the map, with
some counts going into one, and some into another. The
semantics of his program would seem to imply that the map has
identity (or is a member of an object which has identity), so
copying is wrong, period. It's not an optimization issue; the
program will not give the correct results if he e.g. counts the
word using a different copy each time.

Be aware,
however, that if you copy your class, the map will get copied also. So just
make sure you pass your class by reference than by value. One way to ensure
this is to disable copying of your class. The normal way to do this is to
create a copy constructor ( and assignment operator) and make them private
to the class. That way if you accidently write some code that would create
a copy of the class, it won't compile and you will realize it.

Another solution is to derive from a class which inhibits copy
and assignment, something like boost::uncopiable. (In large
applications, this tends to happen more or less automatically,
since most objects with identity will derive from some sort of
EntityObject class, which defines the interface used for
transaction management and/or persistency, and which inhibits
assignment and copy, or restricts them to the transaction
manager. But he's not there yet.)

For the SearchText class that would be:

class SearchText
{
public:
// ...
private:

// Disable copy and assignment by making private.
SearchText( SearchText const&) {}
SearchText& operator=( SearchText const&) {}
}

Just empty copy constructor and assignment operator's private
to the class.

The usual solution is to not define them at all. That way, even
if a member function tries to use them, you get an error at link
time.

 

[James Kanze]

[...]

Just a note. I noticed in that code that in operator= I don't
have a return statement. I looked in my own code where I'm
doing this same thing and also don't have a return statement.
I don't know if that's a compiler flaw not catching it, or the
standard allows it. I think, however, that SearchText&
opeator=( SearchText const& ) { return this; } would be more
correct, even though it will never be called.

Technically, it's undefined behavior *IF* you fall off the end
of the function, and there is no return statement. If you never
call the function, you never fall off the end, and there's no
undefined behavior. Compilers may (and many probably do) warn
about it, however, and at least one compiler I use (Sun CC) does
treat it as an error.

(The reason it's undefined behavior is that it is difficult, if
not impossible, to detect in the general case. Consider
something like:

C&
C:perator=( C const& other )
{
someOtherFunction() ;
}

You may know that someOtherFunction() always calls abort(), or
raises an exception, so that you never fall off the end, but the
compiler really has no guaranteed way of knowing this.)

 

[James Kanze]

One more question though. Why are all you guys constantly typing
'std::something', instead of just typing 'using namespace std' one
time at the top of each file?

Because that's the name of the function, class or whatever. The
'using namespace' directive easily results in confusion, and
should be used very, very sparingly, and never for std:: (which
as namespaces go, is pretty large).