subaruwrx88011 said:
Ok sorry guys I am really new at c++ and how this forum works.
Here is the real code.
class CCSI_EnviromentClass
{
private:
//Constructor
CCSI_EnviromentClass();
typedef struct StateType
{
int i;
char* s;
float f;
StateType() : i(-1), f(1.0) {}
So, you initialise all except 's'. 's' is left _uninitialised_. If
a pointer is uninitialised, it contains _garbage_, it points _nowhere_.
Initialise it to 0, at least then you can _verify_ whether it has some
"real" value or "nothing".
};
typedef std::map<std::string, StateType> ItemStateMap;
ItemStateMap m_itemMap;
static CCSI_EnviromentClass *ms_instance;
public:
//Destructor
~CCSI_EnviromentClass();
The constructor is private, the destructor is public. Is there any
particular reason?
//Methods
static CCSI_EnviromentClass *instance();
int getItemStateInt(char* item);
float getItemStateFloat(char* item);
char* getItemStateString(char* item);
OK... The map is based on 'std::string'. You're passing bare pointers
around. Is there a particular reason?
int setItemState(std::string item, char* state);
int setItemState(std::string item, float state);
int setItemState(std::string item, int state);
void print();
};
char* CCSI_EnviromentClass::getItemStateString(char* item)
{
StateType state;
ItemStateMap::iterator map_iterator;
//Find the item name in the list
map_iterator = m_itemMap.find(item);
//If the item was found then return the state, otherwise return NULL
if(map_iterator != m_itemMap.end())
{
state = m_itemMap[item];
return state.s; //return string part of union
It is better to do
return (*map_iterator).second.s;
here to avoid another search that would inevitably performed when you
use the indexing operator.
}
else
{
return NULL;
}
}
float CCSI_EnviromentClass::getItemStateFloat(char* item)
{
StateType state;
ItemStateMap::iterator map_iterator;
//Find the item name in the list
map_iterator = m_itemMap.find(item);
//If the item was found then return the state, otherwise return NULL
if(map_iterator != m_itemMap.end())
{
state = m_itemMap[item];
return state.f; //return float part of union
Same note as above
}
else
{
return -1;
}
}
int CCSI_EnviromentClass::getItemStateInt(char* item)
{
StateType state;
ItemStateMap::iterator map_iterator;
//Find the item name in the list
map_iterator = m_itemMap.find(item);
//If the item was found then return the state, otherwise return NULL
if(map_iterator != m_itemMap.end())
{
state = m_itemMap[item];
return state.i; //return float part of union
And here, too.
}
else
{
return -1;
}
}
int CCSI_EnviromentClass::setItemState(std::string p_item, char*
p_state)
{
StateType state;
state.s = p_state;
m_itemMap[p_item] = state;
return 0;
}
int CCSI_EnviromentClass::setItemState(std::string p_item, float
p_state)
{
StateType state;
state.f = p_state;
m_itemMap[p_item] = state;
return 0;
}
int CCSI_EnviromentClass::setItemState(std::string p_item, int
p_state)
{
StateType state;
state.i = p_state;
m_itemMap[p_item] = state;
return 0;
}
Hope this is better.
Actually, just a tiny bit better. But of course, since you never show
_who_ uses those functions (especially the suspect 'setItemState') and
_how_ they are used (what is passed to them), there is no way to tell,
again. The code is real alright. However, it's _incomplete_.
V
