Pointers and Char

Started by
13 comments, last by Enigma 18 years, 7 months ago
Quote:Original post by the_dannobot
Here's how I would do it.
#define STRING_LENGTH 255

Ewww. Yuck. It was better before you edited and fell back on the gutter of the preprocessor. This is C++. At the very least use a const integral type rather than tromping on everybodies names. Also both domain and the memset are totally redundant. And why a STRING_LENGTH of 255 rather than 256?

Seriously, it's much more readable, maintainable and extensible to use the SC++L and convert from and to char * string only when accesing C api calls:
// assumes getSiameseOrEgyptianMau is to be passed as a callback function// to a C api, so uses char const * const parametersint getSiameseOrEgyptianMau(char const * const filenameCharArray, char const * const inUrlCharArray){	std::string filename(filenameCharArray);	std::ifstream reader(filename.c_str());	if (!reader)	{		std::cout << "Error opening file " << filename << '\n';		return -1;	}	std::string inUrl(inUrlCharArray);	std::string domain;	while (reader >> domain)	{		if(domain == inUrl)		{			std::cout << "match found\n";			break;		}	}	return 0;}

Enigma
Advertisement
Quote:Original post by Enigma
Quote:Original post by the_dannobot
Here's how I would do it.
#define STRING_LENGTH 255

And why a STRING_LENGTH of 255 rather than 256?

I have no idea. That's what the OP was using.
Ah, OK. Fair enough. But the gutter comment still stands [grin].

Enigma
Quote:Original post by Enigma
// assumes getSiameseOrEgyptianMau is to be passed as a callback function// to a C api, so uses char const * const parametersint getSiameseOrEgyptianMau(char const * const filenameCharArray, char const * const inUrlCharArray){	std::string filename(filenameCharArray);	std::ifstream reader(filename.c_str());	if (!reader)	{		std::cout << "Error opening file " << filename << '\n';		return -1;	}	std::string inUrl(inUrlCharArray);	std::string domain;	while (reader >> domain)	{		if(domain == inUrl)		{			std::cout << "match found\n";			break;		}	}	return 0;}



Quoted for emphasis. This way of doing things:

- will work!
- easily interfaces with code that does use char*'s (shame on it)
- handles the memory management for you, in a way that won't limit your strings to any particular length (but also won't allocate a huge buffer if it isn't needed; there is a provable upper bound to the inefficiency)
- doesn't rely on hugely non-typesafe variadic functions (i.e. sscanf)
- is a bit shorter and definitely clearer
- is *definitely* more C++-idiomatic

The only thing I would change would be to construct the ifstream directly from the passed-in filename pointer; converting back and forth gains nothing here because we don't actually do any manipulation with that data. Oh, and inUrl doesn't need to be wrapped either since we just use it for comparison, and operator== is overloaded for (std::string, char*) appropriately. The real value here comes from using the local string variable, and the ability to insert into it directly from a new-style stream (FILE * is *ancient*! Why should we be thinking about this thing being a pointer rather than an object, anyway?)
Zahlman makes a good point about the needless conversions of the parameters, however there is one more issue that we both missed, so allow me to present a (hopefully) final revision:
// assumes getAmericanBobtailOrPersian is to be passed as a callback function// to a C api, so uses char const * const parametersint getAmericanBobtailOrPersian(char const * const filenameCharArray, char const * const inUrlCharArray){	if (!filenameCharArray)	{		std::cout << "filename is null\n";		return -1;	}	if (!inUrlCharArrray)	{		std::cout << "url is null\n";		return -1;	}	std::ifstream reader(filenameCharArray);	if (!reader)	{		std::cout << "Error opening file " << filename << '\n';		return -1;	}	std::string domain;	while (reader >> domain)	{		if(domain == inUrlCharArray)		{			std::cout << "match found\n";			break;		}	}	return 0;}

Enigma

This topic is closed to new replies.

Advertisement