Back to General and Gameplay Programming

Setting Chars

General and Gameplay Programming Programming

Started by Mr_Fhqwhgads November 16, 2006 12:27 AM

12 comments, last by Aardvajk 17 years, 5 months ago

Mr_Fhqwhgads

100

Author

November 16, 2006 12:27 AM

I am trying to set a char(or string, if thats better) to somethign from the constructor.


	Cvar( const char *aName, const char *aValue, int aFlags ) 
	  {
		  strcpy( name, aName);
		  strcpy( sValue, aValue);
		  flags = flags;
	  }

The part where it does the strcpy throws a runtime error saying ti can't access memory. I have tried strings, changing it to/from pointers, not exacly sure whats wrong with it.


class Cvar
{
public:
	Cvar *next;
	char *name, *sValue;
	int flags;

	Cvar( const char *aName, const char *aValue, int aFlags ) 
	  {
		  strcpy( name, aName);
		  strcpy( sValue, aValue);
		  flags = flags;
	  }
	 //char *GetValue();
};

Blupix Games

Anonymous

November 16, 2006 12:37 AM

You have horrid variable and class naming habits, unless you plan to work alone forever get into better habits...

class Cvar{public:	Cvar *next;	char *name, *sValue;	int flags;	Cvar( const char *aName, const char *aValue, int aFlags ) 	  {                  name = new[GetLength(aName)];                  sValue = new[GetLength(aValue)];		  strcpy( name, aName);		  strcpy( sValue, aValue);                  // This was flags = flags which was wrong...		  flags = aFlags;	  }        ~Cvar()          {              delete [] name;              delete [] sValue;          }	 //char *GetValue();};A better solution...class Cvar{public:	Cvar *next;	std::string name, sValue;	int flags;	Cvar( const std::string aName, const std::string aValue, int aFlags ) 	  {                  name = aName;                  sValue = aValue;                  // This was flags = flags which was wrong...		  flags = aFlags;	  }        ~Cvar()          {          }	 //char *GetValue();};

There, isn't that much better, cleaner, easier? :)

Mr_Fhqwhgads

100

Author

November 16, 2006 12:41 AM

yea...I know. I was just writing this really quick to see if I could get it to work. Any tips on naming conventions? Thanks.

Blupix Games

blarn

128

November 16, 2006 02:00 AM

aw I don't think the naming done is bad. I've seen (and sometimes use) structs with a member variable 'name' with a constructor that takes an argument 'theName', which isn't much different from using 'name' and 'aName' like in the example. I think it's more sane reading 'somestruct.name' instead of 'somestruct.m_name'.

(Although, I usually only do this though when the struct is only holding data, and doesn't have any real functionality other than that. For classes that actually do more complicated things I use the 'm_foo' convention for member variables to make it easier to understand the scope of variables in member functions.)

ANYWAYS! different strokes for different folks?

if you go with the std::string implementation you should use initialization lists too, so

Cvar( const std::string aName, const std::string aValue, int aFlags ) : name( aName ), sValue( aValue ), flags( aFlags ){}

Aardvajk

13,217

November 16, 2006 02:45 AM

If you really want to take advantage of the STL,

struct ValueType{    std::string Value; int Flags;    ValueType(const std::string &V="",int F=0) : Value(V),Flags(F) { }};typedef std::map<std::string,ValueType> ValueTable;ValueTable Vars;void f(){    Vars["a"]=ValueType("hello",23);    cout << Vars["a"].Value;}

would be my suggestion, although I'm second-guessing what you are trying to do a bit.

journal | website | twitter | youtube

Julian90

736

November 16, 2006 02:48 AM

some more refactoring :-)

// Close to your version (see below for a better version)class Var{public:    MyClass *Next;    std::string Name;    std::string Value;    int Flags;    // fixed this to pass by reference, also why use aName or TheName    // when you can just use name? even if you used Name saying    // Name(Name) in the initializer list is still unambiguise    // and you can use this->Name in the body to resolve ambiguity    Var(const std::string& name, const std::string& value, int flags):      Name(name),      Value(value),      Flags(flags)    {    }};// This seems far more logical IMO but i had to assume a few things about// what you were doingclass Data{public:    Var(const std::string& name, const std::string& value, int flags):      mName(name),      Value(value),      mFlags(flags)    {    }    const std::string& GetName();    void FlipFlag(int flagNum);    void SetFlag(int flagNum, bool value);    // do you ever logically need to get the flags?    // if so what about this instead    bool GetFlag(int flagNum);public: // just personal preference to start a new section for variables    std::string Value; // should this really be a string maybe std::vector<char>private:    const std::string mName; // Does it really make sense for this to be able to change?    int mFlags;};// also you probally noticed i got rid of your Cvar* next, thats because// it seems to me like thats being used in a linked list style so just// make a std::list<Cvar> when you want to use some Cvar's :-)

i dont think theres much more i can say without some context so bye

Edit: hmm i prefer what EasilyConfused (i missed that interpretation) said but im not sure that either that or what i said are what your trying to do.

Random Link

Evil Steve

2,021

November 16, 2006 03:36 AM

The reason it doesn't work, is that you're trying to strcpy() into something that isn't allocated yet. You should either just copy the pointer (Which is usually a bad idea, since the caller might have the buffer passed to the function on the stack), or to allocate memory in the class, either with new[] or by using a static buffer, like char sValue[256];.

Although, if this is C++, you should definitely go for the std::map of std::strings idea. It'll be faster, far less error prone, and easier to write and maintain.

Zahlman

1,682

November 16, 2006 04:14 AM

That "next" pointer makes me *very* suspicious. Are you building your own linked list? Don't do that; the standard library already provides a perfectly good one, std::list.

Although, if you're going to be doing all your lookup by searching for the "name", as EasilyConfused is guessing, then you instead want std::map. That is a data container that actually implements those semantics directly (lookup by a 'key'), and will provide faster searches than are possible with a linked list (because it internally sorts the data and can find the item by binary search; you *could* binary-search a doubly-linked list, but it isn't any faster than sequential search on a doubly-linked list).

And oh, yeah. std::string, and initializer lists. I don't know what language you've been using, but welcome to C++.

Basically, what EasilyConfused said. Except that you don't even need to define a "value type" for something this simple, because again, the standard library provides:

typedef std::pair<std::string, int> ValueType;// That hard-wires the field names, because there's no way you could specify// them via the template. .first replaces .Value; .second replaces .Flags .// But it saves you all that class creation work :)typedef std::map<std::string, ValueType> ValueTable;ValueTable Vars;void f() {  Vars["a"] = ValueType("hello", 23);  cout << Vars["a"].first;}

There, isn't that much better, cleaner, easier? ;)

Damon Shamkite

139

November 16, 2006 04:17 AM

Quote:Original post by Evil Steve
The reason it doesn't work, is that you're trying to strcpy() into something that isn't allocated yet.

Finally, someone actually addressing the problem, and answering the question :)

Aardvajk

13,217

November 16, 2006 04:46 AM

Quote:Original post by Damon Shamkite
Quote:Original post by Evil Steve
The reason it doesn't work, is that you're trying to strcpy() into something that isn't allocated yet.
Finally, someone actually addressing the problem, and answering the question :)

Um... the AP did that on the first reply. I'm not knocking Steve's clarification, of course, but I think once the problem has been answered in simple terms it is fair game to suggest improvements.

journal | website | twitter | youtube

Setting Chars

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Setting Chars

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines