seeking an opinion from the collective wisdom here

[Devon Null]

[edited to try and eliminate word wrapping in code]

I was simply wondering if this was a mal-formed class header:

#ifndef _items_h_
#define _items_h_

#include <iostream>
#include <string>
#include <vector>

using namespace std;

class Item
{
public:
Item( float weight = 0,\
bool set_can_equip = false,\
bool set_can_attack = false,\
bool set_can_defend = false,\
bool set_can_drink= false,\
bool set_can_eat = false,\
bool set_can_use = false,\
bool set_is_consumab le = false,\
bool set_is_magical = false );
//Below are the set and get functions for
//all variables in this class

void set_weight( float weight )
{ item_weight = weight; }
float get_weight()
{ return item_weight; }

void set_can_equip( bool set_can_equip )
{ can_equip = set_can_equip; }
bool get_can_equip()
{ return can_equip; }

void set_can_attack( bool set_can_attack )
{ can_attack = set_can_attack; }
bool get_can_attack( ) { return can_attack; }

void set_can_defend( bool set_can_defend )
{ can_defend = set_can_defend; }
bool get_can_defend( ) { return can_defend; }

void set_can_drink( bool set_can_drink )
{ can_drink = set_can_drink; }
bool get_can_drink() { return can_drink; }

void set_can_eat( bool set_can_eat )
{ can_eat = set_can_eat; }
bool get_can_eat() { return can_eat; }

void set_can_use( bool set_can_use )
{ can_use = set_can_use; }
bool get_can_use() { return can_use; }

void set_is_consumab le( bool set_is_consumab le )
{ is_consumable = set_is_consumab le; }
bool get_is_consumab le() { return is_consumable; }

void set_is_magical( bool set_is_magical )
{ is_magical = set_is_magical; }
bool get_is_magical( ) { return is_magical; }

void describe(); // simply returns description (see below)
void clear_describe( ); // clears description
void add_describe( string line_to_add );
/*Line above checks to see if description is set to either
need_set or desc_cleared, and if so description.cle ar()
then push_back( line_to_add ), else just
push_back( line_to_add)*/

private:
// These are stats that will be set for every item created
string need_set; // description is initialized with this
string desc_cleared; // description will be set to this
//after it has been cleared
vector<stringde scription;
// This is to allow multi-line descriptions
float item_weight; // These are pretty self explanatory I think
bool can_equip;
bool can_attack;
bool can_defend;
bool can_drink;
bool can_eat;
bool can_use;
bool is_consumable;
bool is_magical;
};

#endif

but as I read along I was confused over something. The above works just
fine, but I was also wondering should the constructor be this:

Item( float weight = 0,\
bool set_can_equip = false,\
bool set_can_attack = false,\
bool set_can_defend = false,\
bool set_can_drink = false,\
bool set_can_eat = false,\
bool set_can_use = false,\
bool set_is_consumab le = false,\
bool set_is_magical = false );

with a constructor body like this in the cpp file:

Item::Item( float weight,\
bool set_can_equip,\
bool set_can_attack, \
bool set_can_defend, \
bool set_can_drink,\
bool set_can_eat,\
bool set_can_use )
{
item_weight = weight;
can_equip = set_can_equip;
can_attack = set_can_attack;
can_defend = set_can_defend;
can_drink = set_can_drink;
can_eat = set_can_eat;
can_use = set_can_use;
is_consumable = set_is_consumab le;
is_magical = set_is_magical;
need_set = "You need to set a description here!\n";
desc_cleared = "The description has been cleared.\n";
description.cle ar();
description.pus h_back( need_set );
}

or this:

Item() : float weight( 0 ),\
bool can_equip( false ),\
bool can_attack( false ),\
bool can_defend( false ),\
bool can_drink( false ),\
bool can_eat( false ),\
bool can_use( false ),\
bool is_consumable( false ),\
bool is_magical( false );

and lose the entire body of the constructor in the cpp file? I will need
to pass in parameters to override the defaults when I instantiate the
objects.

Question 1: Is this a mal-formed class header?
Question 2: Which should I use for the initialization list?

Thanks in advance.
DN
--
[there are no x's in my email]

I have the right to remain silent
(and should probably use it as much as possible)
Anything I type can and will be used against me
in a court of idiocy
I have the right to be wrong
(and probably am)
If I can not furnish my own wrongness
I'm sure someone will provide it for me.

[Devon Null]

Jim Langston wrote:

The users of the class shouldn't worry about what my variable is called.
Drinkable is already a boolean concept (yes or no) so why should I have to
say IsDrinkable? And if I want to set it, why have to say SetDrinkable?
Doesn't Drinkable( true ) express that? I saw the trailing underscores used
and found it the best to my liking, so I use it.

Thank you for explaining that. I thought maybe it was a matter of the
"accepted" way of doing things. If I understand you correctly it is a
matter of personal taste. That is the reason for the name is_drinkable
and such like. I ask myself whether an item is_drinkable. Just an easier
way to remember it (for me.) I did adopt the underscore idea though.
Even with overloading and the such-like, I don't really feel comfortable
naming more than one object the same names unless it makes operational
sense. This pointed that out to me.

Thanks again
DN
--
[there are no x's in my email]

I have the right to remain silent
(and should probably use it as much as possible)
Anything I type can and will be used against me
in a court of idiocy
I have the right to be wrong
(and probably am)
If I can not furnish my own wrongness
I'm sure someone will provide it for me.

[=?iso-8859-1?q?Kirit_S=E6lensminde?=]

On Jun 3, 2:24 am, Devon Null <theronnights.. [e-mail] ote:

void set_weight( float weight )
{ item_weight = weight; }
float get_weight()
{ return item_weight; }

etc. etc.

There's a huge amount of redundant code for all of these accessors.
You can use a public templated functor object to provide all of this
for free. Personally I wouldn't use the set_ or get_ in C++ either.

This sort of thing will do it (not run through a compiler):

template< typename T >
class Accessor {
T m_t;
public:
Accessor() : m_t() {}
Accessor( const T &t ) : m_t( t ) {}

const T &operator() () const { return m_t; }
void operator() ( const T &t ) { m_t = t; }
};

class Item {
public:
Accessor< float weight /* etc. */;
Accessor< bool can_equip, can_attack /* etc. */;
};

http://www.kirit.com/C%2B%2B%20kille...et%20accessors
K

[Puppet_Sock]

On Jun 2, 3:24 pm, Devon Null <theronnights.. [e-mail] ote:
[snip]

class Item
{
public:
Item( float weight = 0,\
bool set_can_equip = false,\
bool set_can_attack = false,\
bool set_can_defend = false,\

Heh heh. "There are twisty passages in the immediate vicinity."

[snips]

//Below are the set and get functions for
//all variables in this class

void set_weight( float weight )
{ item_weight = weight; }
float get_weight()
{ return item_weight; }

Another poster has already talked about all these
get-set functions.

Think about this object in a different way. What is
the purpose of this object in your program? What
service does it provide? What job does it do?

Sometimes it is appropriate to have that job be,
in effect, "carry around all these parameters like
a big basket full of data, hold them up when asked,
and take new values when asked." And if that is the
task, then certainly get-set may be appropriate.
And the template method of doing them may be a good
idea as well.

But the name, and some of the member variable names,
suggest you may be missing a chance for some good
object oriented design practice. What about making
that job description be a bit more closely related
to the task your application is intended for. How
about something like: Act as one object that the
character can manipulate?

Then you might not want all the get-set functions.
Instead, you may want things like

bool PutInPack(){// etc.}

with appropriate conditions and reports. Maybe it does
it if it can, but if it can't it leaves it where it
was and reports to the user "you can't put that in
your backpack." Or a function

bool Eat(){// etc.}

and if it works, the object is removed (destroyed)
but if it fails, it reports "You can't eat that!"
or something. Or maybe it should be

int Eat(){// etc.}

and return the number of turns of food that worked
out to, with reporting the result. And so on. Maybe
eating an apple gives you, say, 10 turns of food
and converts the apple into an apple core.

If you do all that, it's quite likely that you
won't ever call many of those get-set functions.
And you will start to think about where the code
to do something like DefendWith() belongs. Which
may (or may not) be in the Item class.

You will also start to think about which classes
have to know about which other classes. Does the
Item class have to know about the Backpack class?
Or does the Backpack class have to know about the
Item class? Or neither? Or both? And that gets you
into lots of other good OOP practice.
Socks

[Devon Null]

Puppet_Sock wrote:

Think about this object in a different way. What is
the purpose of this object in your program? What
service does it provide? What job does it do?

Sometimes it is appropriate to have that job be,
in effect, "carry around all these parameters like
a big basket full of data, hold them up when asked,
and take new values when asked." And if that is the
task, then certainly get-set may be appropriate.
And the template method of doing them may be a good
idea as well.

That is exactly my goal. This is meant to be a base class for all
objects in the game that could be called items - weapons, armor, food,
potions, scrolls, etc. I would have made this pure virtual if I could
think of a function that would need to be defined differently for all
the derived classes. If I figure it out I will add it.

>
What about making that job description be a bit more closely related
to the task your application is intended for. How
about something like: Act as one object that the
character can manipulate?

That is the job of my derived classes, at least in the class hierarchy
in my head.

Then you might not want all the get-set functions.

I use the get and set names simply for ease of remembrance on my part.
To me (the only one I am writing code for ATM) they are self-explanatory.

>
bool PutInPack(){// etc.}

The Bag base class I came up with takes care of that. (See the post
titled "Am I reinventing the wheel?")

>
with appropriate conditions and reports. Maybe it does
it if it can, but if it can't it leaves it where it
was and reports to the user "you can't put that in
your backpack." Or a function

bool Eat(){// etc.}

and if it works, the object is removed (destroyed)
but if it fails, it reports "You can't eat that!"
or something. Or maybe it should be

int Eat(){// etc.}

and return the number of turns of food that worked
out to, with reporting the result. And so on. Maybe
eating an apple gives you, say, 10 turns of food
and converts the apple into an apple core.

I was going to handle that outside of any classes, but I might have to
rethink that. I may add that to a Food class or the such like. Maybe
that (replace item with another item after certain condition are met)
will be my pure virtual function.

You will also start to think about which classes
have to know about which other classes. Does the
Item class have to know about the Backpack class?
Or does the Backpack class have to know about the
Item class? Or neither? Or both? And that gets you
into lots of other good OOP practice.
Socks

See my last comment, but this is the kind of advice and direction I can use.

I do appreciate all the constructive advice I am getting. I know it is
ambitious to use something of this complexity (the game as a whole) as a
way to learn the language, but I have tried other ways and this one
seems to be working. I have learned a great many concepts, and others
FINALLY make sense (i.e. classes).

Thanks again for everyone's help
DN
--
[there are no x's in my email]

I have the right to remain silent
(and should probably use it as much as possible)
Anything I type can and will be used against me
in a court of idiocy
I have the right to be wrong
(and probably am)
If I can not furnish my own wrongness
I'm sure someone will provide it for me.

[Puppet_Sock]

On Jun 7, 4:40 pm, Devon Null <theronnights.. [e-mail] ote:
[snip]

I was going to handle that outside of any classes,
but I might have to rethink that.

The more C++ I write, the less code I put outside
of classes. The most recent app I wrote had nearly
everything inside classes. The main function checked
there were the right number of user inputs, created
the object that did most of the work, then called that
object's "do most of the work" function. Altogether
about 10 lines of code.

Now there were quite a few utility functions. But
these were entirely generic. So, things like I was
noticing I was doing a particular format conversion
in many places, so I made a func to do that.

And I'm getting more into the habit of, and getting
more adept at, using templates and standard library
features. So the number of util funcs I write in a
code is falling also.
Socks

[Devon Null]

Puppet_Sock wrote:

>
And I'm getting more into the habit of, and getting
more adept at, using templates and standard library
features. So the number of util funcs I write in a
code is falling also.
Socks

Templates are a weak point for me (read: I know jack about them.) I am
trying to use as many standard library features as I can. I add them as
I learn them. Unfortunately the only person I have to teach me the vast
majority of C++ is myself, both usage and the language itself. Kind of
like writing a debugger and using it to debug itself.

DN

--
[there are no x's in my email]

I have the right to remain silent
(and should probably use it as much as possible)
Anything I type can and will be used against me
in a court of idiocy
I have the right to be wrong
(and probably am)
If I can not furnish my own wrongness
I'm sure someone will provide it for me.