Sign in to follow this  

Less juggling between string, stringstream, and char* ?

This topic is 3871 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi, I'm using some libraries (HGE) to read from an ini file, and the HGE functions always use char* as strings, so you have to put in a char* to tell it what ini item to get, and it will return the item's value as a char*. I have a bunch of ini items that I expect the user to put in the ini file, and name them sequentially. Soo... my understanding is that I need to use a stringstream to concatenate strings and numbers together to form a string. Eg. Path1, Path2, Path3, etc. My issue is further complicated by the fact that I could not get strcmp() to sucessfully check if a char* equals "", it just always returned false. eg if(strcmp(charPointer, "")) - so I have to throw in std::string into the mix just for the sake of checking if the string is "". I want to check for "" to end the loop, since the last parameter for HGE's GetString function is where you specify a default value if the ini item is not found, and I make the default value "", so I know that if I get "" I can stop checking for more ini items. So here is the resulting code... it works, but LOTS of juggling between string formats and I can't help but wonder if it could be neater. All this code does is;
-Initialise the loop by getting the value of item Path1 from the ini file
-Loop while the value from the ini file is not ""
    -Store the value in a vector
    -Get the value of the next item (eg. Path2, Path3, etc.) from the ini file
	std::stringstream stream;
	int i = 1;
	char *iniValue;
	std::string stringIniValue;
	stream << "Path" << i;
	iniValue = m_hge->Ini_GetString("Rom Paths", stream.str().c_str(),"");
	stringIniValue = iniValue;
	while (stringIniValue != "")
	{
		m_romPaths.push_back(stringIniValue);
		stream.str("");
		i++;
		stream << "Path" << i;
		iniValue = m_hge->Ini_GetString("Rom Paths", stream.str().c_str(),"");
		stringIniValue = iniValue;
	}

Share this post


Link to post
Share on other sites
std::string getString(const std::string & str)
{
return m_hge->Ini_GetString("Rom Paths", str.c_str(),"");
}

void theFunction()
{
const std::string path = "Path";
for (int i = 0;;++i)
{
std::string obtained = this->getString
(path + boost::lexical_cast<std::string>(i));
if (obtained == "") return;
m_vector.push_back(obtained);
}
}

Share this post


Link to post
Share on other sites
strcmp returns 0 if the strings equal, so you really want if(!strcmp(charPointer, "")) to check for an empty string. Alternatively you can check if the first character is null: if(*charPointer == 0).

Using std::string is usually easier an safer however.

Share this post


Link to post
Share on other sites

std::stringstream stream;
int i = 1;
char *iniValue;
bool ValidString = true;
do
{
stream << "Path" << i;
iniValue = m_hge->Ini_GetString("Rom Paths", stream.str().c_str(),"");
if(!strcmp(iniValue, ""))
{
m_romPaths.push_back(string(iniValue));
stream.str("");
++i;
}
else
{
ValidString = false;
}
} while(ValidString);



I think that should work.

Share this post


Link to post
Share on other sites
This place is awesome - I get like 4 replies within 20 minutes of posting, so it's like live help, but 4x better.

sphen_lee, Alpha_ProgDes - thanks for pointing that out. That was quite stupid of me.

ToohrVyk, f8k8 - both examples left me with an empty vector, though my old code still works. Here is how I modified them to compile;

ToohrVyk;

std::string RomPaths::GetStringFromIni(const std::string & str)
{
//Ini_GetString always returns a char*
return boost::lexical_cast<std::string>(m_hge->Ini_GetString("Rom Paths", str.c_str(),""));
}




const std::string path = "Path";
for (int i = 0;;++i)
{
std::string obtained = this->GetStringFromIni(path);
(path + boost::lexical_cast<std::string>(i));
if (obtained == "") return;
m_romPaths.push_back(obtained);
}




f8k8, all I did was change
m_romPaths.push_back(string(iniValue));
to
m_romPaths.push_back(std::string(iniValue));


I hope you can tell me why they give me no results, but in any case I have learned something about how to do things, from those examples.

Share this post


Link to post
Share on other sites
Not entirely on topic:
Quote:
Original post by ToohrVyk
if (obtained == "") return;

Wouldn't it be clearer and less wasteful to test obtained.empty()?

Admiral

Share this post


Link to post
Share on other sites
Quote:
Original post by Domarius
ToohrVyk, f8k8 - both examples left me with an empty vector, though my old code still works. Here is how I modified them to compile;


The char* to string is my fault. You could use an explicit std::string constructor instead of lexical_cast, however. Also, my i = 0 should probably be i = 1.

TheAdmiral: it would certainly be less wasteful (although one might wonder if operator==(std::string,char*) is not overloaded and results in a very early-out), and I honestly don't know which version is the most idiomatic in C++ (the str == "" being the idiomatic one in most other languages I use).

Share this post


Link to post
Share on other sites
Quote:
Original post by Domarius
ToohrVyk, f8k8 - both examples left me with an empty vector, though my old code still works. Here is how I modified them to compile;


const std::string path = "Path";
for (int i = 0;;++i)
{
std::string obtained = this->GetStringFromIni(path);
(path + boost::lexical_cast<std::string>(i));

// Notice that the second line concatenates the converted-to-string
// number to the path name, but then just throws that result away.
// The GetStringFromIni() function is simply called with 'path'.
if (obtained == "") return;
m_romPaths.push_back(obtained);
}


From ToohrVyk's original code:


std::string obtained = this->getString // <-- notice there is no ; here!
(path + boost::lexical_cast<std::string>(i));
// Thus the expression in brackets is the argument for the function call.





My offering, based on what has been seen so far:


std::string RomPaths::GetStringFromIni(const std::string& name, int value) {
std::string path = name + boost::lexical_cast<std::string>(value);
return std::string(m_hge->Ini_GetString("Rom Paths", path.c_str(), ""));
}

const std::string path = "Path";
int i = 0;
do {
m_romPaths.push_back(GetStringFromIni(path, ++i));
} while (!m_romPaths.back().empty());
m_romPaths.pop_back();


Although something still bothers me here: this kind of wrapping should be done as "close to the interface" as possible. The *real* way to deal with HGE's old-style interface is to wrap the HGE objects themselves. Something like


// Needs a much better name. What is it that this thing *does*?
class HGEWrapper {
boost::shared_ptr<foo> m_hge; // whatever type it is that HGE exposes.

public:
std::string getIniString(const std::string& x, const std::string& y, const std::string& z) {
return std::string(m_hge->Ini_GetString(x.c_str(), y.c_str(), z.c_str()));
}

// etc. etc. etc.
};

// Now RomPaths holds an HGEWrapper by value instead, and:

const std::string path = "Path";
int i = 0;
do {
std::string name = path + boost::lexical_cast<std::string>(++i);
m_romPaths.push_back(m_hgeWrapper.getIniString("Rom Paths", name, ""));
} while (!m_romPaths.back().empty());
m_romPaths.pop_back();

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
std::string obtained = this->getString // <-- notice there is no ; here!


OMG I am such a twit! I "corrected" it thinking it was a typo :(

To answer your question, the hge variable (m_hge in my class) is a singleton. Every where you need it in scope, you go
m_hge = hgeCreate(HGE_VERSION);
and it returns a pointer to the same object, and (AFAIK) increments a counter. When you're done with it, you call
m_hge->Release();
and it decrements the counter, and finally destroys the object when there are no more pointers to it.
Of course I didn't make this awesome peice of work, this is just how the HGE library works :)

I like your idea of wrapping the entire class so I don't have to do the conversions in my app :) I'm gonna see what they think about that at the HGE forum, and ask why char* is used anyway. The HGE object has a lot of functions that you call from it - I suppose I could just add them to the wrapper as I need them. I'm gonna think about this.

Your code snippit worked straight out of the box, just thought I would let you know :) I'm keeping it there, unless I decide to make this wrapper.

Thanks everyone for your help. I learned quite a bit here. :)

Share this post


Link to post
Share on other sites
Quote:
Original post by Domarius
To answer your question, the hge variable (m_hge in my class) is a singleton.


Oh.

The best "wrapping" might then not use any objects at all, then. Instead, you would create a namespace with free functions for all the functionality you want to "reflect", and just call hgeCreate (and use the result) from within the free functions (presumably this function will return early when the singleton already exists :) ). This is actually likely to save you significant amounts of memory (in the form of redundant pointers to the singleton that are no longer stored all over the place).

Quote:
and it returns a pointer to the same object, and (AFAIK) increments a counter. When you're done with it, you call
m_hge->Release();
and it decrements the counter, and finally destroys the object when there are no more pointers to it.


Oh. That complicates things, then. But there's hope. Something like this should do it:

HGEWrapper.h:

#ifndef HGE_WRAPPER_H
#define HGE_WRAPPER_H
namespace my_hge {
void foo();
}
#endif



HGEWrapper.cpp:

namespace my_hge {

namespace detail {
// We define an object that uses RAII to control a pointer to the singleton:
class HGEWrapper {
HGEInterface* ptr;

// Stubbing these out prevents the object from being copied, which is
// what we want, here.
HGEWrapper(const HGEWrapper&); // don't try to implement this.
HGEWrapper& operator=(const HGEWrapper&); // don't try to implement this.

public:
HGEWrapper(int version) : ptr(HGECreate(version)) {}
~HGEWrapper() { ptr->Release(); }
HGEInterface& operator*() { return *ptr; }
HGEInterface* operator->() { return ptr; }
} implementation;
// This ^^^ instance of the wrapper object ensures that the counter is at
// least 1 during the entire program, so that the thing won't be re-created
// continually. Note that you might have to worry about the "static order
// of initialization fiasco" here, if you're unlucky enough that there are
// other singletons in the program and they have dependencies on each other.
}

void foo() {
// Because we had to make that static instance, we just use it in the
// function implementations - although we could also create our own local
// instances and everything would be just fine.
// Anyway, we just forward things to the singleton like this:
detail::implementation->foo();
}
}






On the other hand, it sort of depends on the scope of HGE. If it tries to do too much, there may be cause to split the functionality up into several headers; and there might yet be some use for actual objects (of the sort that can be freely instantiated) - e.g. to store parameters for a later call to a complicated function.

Share this post


Link to post
Share on other sites
Well we never use "new" to create a hge object. We just go
#include "hge.h"
HGE *hge = hgeCreate(HGE_VERSION);

and hge is now loaded with a hge object.

Is there any reason why my wrapper couldn't just have a hgeCreate function that just goes

HGE myHgeCreate(whatever)
{
return hgeCreate(HGE_VERSION);
}

I would ask on the hge forums, and I have, but with no response, and you seemed to think it would complicate things, so I'm curious as to why :)

Share this post


Link to post
Share on other sites
google "RAII" - it stands for Resource Acquisition is Initialization. Its a long name for saying "grab resources in constructors - release them in destructors". By doing this you are more exception-safe. If you wrap those calls up in the constructor/destructor then your interface is acquired automatically whenever you instantiate an object - and it is released properly automatically whenever that object falls out of scope and is destroyed. If, for example, you directly call your createInterface function and then an exception is fired off you are not guaranteed to release the interface - but if that call is in your destructor it will be released as the stack unwinds and your object falls out of scope. This could also come up if you return out of a function earlier than you expect (an error condition maybe ).

Share this post


Link to post
Share on other sites

This topic is 3871 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this