When can one use "bare" procedural code?

[mike3]

Hi.

I was wondering where, if at all, it's "OK" to use "bare" procedural
stuff (i.e. not part of objects, but bare functions) in C++. For
example, I've got these routines that implement a random level
generator algorithm (for a game). Is it OK to just do this
procedurally, like have a "MakeTunnel" function to make tunnels in the
map (which is an object, by the way), a "MakeRoom" function to make
rooms, etc., or should one put that in objects of some kind? If so,
what kind of way would be good for this? This one seems tricky...

 

[tonydee]

I was wondering where, if at all, it's "OK" to use "bare" procedural
stuff (i.e. not part of objects, but bare functions) in C++.

It's perfectly ok in general... good to do unless there's a reason not
to. OO has many benefits though, both in elegant notation and in
encapsulation and functionality, that there's often a reason.

For
example, I've got these routines that implement a random level
generator algorithm (for a game). Is it OK to just do this
procedurally, like have a "MakeTunnel" function to make tunnels in the
map (which is an object, by the way), a "MakeRoom" function to make
rooms, etc., or should one put that in objects of some kind? If so,
what kind of way would be good for this? This one seems tricky...

You haven't given us enough information to give good advice. map is
an object, but are tunnel or room objects? If so, is there just one
Tunnel and Room class to deal with, or whole hierarchies of
polymorphic variants? In the former case, you might want to have
Tunnel and Room constructors instead of MakeTunnel/MakeRoom; in the
latter, MakeTunnel and MakeRoom might be valid free-standing (or
static member) Factory functions (a Design Pattern common in OO
programming). But, if MakeTunnel and MakeRoom aren't making any kind
of object/struct, then they'll probably be standalone functions. As
the design is fleshed out, many considerations come into play....

Cheers,
Tony

 

[mike3]

It's perfectly ok in general... good to do unless there's a reason not
to.  OO has many benefits though, both in elegant notation and in
encapsulation and functionality, that there's often a reason.


You haven't given us enough information to give good advice.  map is
an object, but are tunnel or room objects?  If so, is there just one
Tunnel and Room class to deal with, or whole hierarchies of
polymorphic variants?  In the former case, you might want to have
Tunnel and Room constructors instead of MakeTunnel/MakeRoom; in the
latter, MakeTunnel and MakeRoom might be valid free-standing (or
static member) Factory functions (a Design Pattern common in OO
programming).  But, if MakeTunnel and MakeRoom aren't making any kind
of object/struct, then they'll probably be standalone functions.  As
the design is fleshed out, many considerations come into play....

Tunnels and rooms are just patterns drawn on the map grid. The
routines
I need will draw those patterns onto the grid. The "MakeTunnel" and
"MakeRoom" routines would just draw those patterns to the map grid.
The
"MakeRoom" routine, for instance, would need to take coordinates for
the
corners of the room's bounding rectangle, which exit points should
have
doorways stuck in them, and of course the type of room to draw on the
grid (which determines how many exit points there are (for some room
types,
this number is proportional to the room's size) and the shape of the
room
drawn inside the rectangle).

 

[Stefan Ram]

I was wondering where, if at all, it's "OK" to use "bare" procedural
stuff (i.e. not part of objects, but bare functions) in C++.

Feel free to do whatever you deem appropriate. Sometimes, people are
successful by ignoring some of the »rules«.

Sometimes, during the process, you might see favorable refactorings
that you can not see yet, which sometimes then might transform
the code into something »object oriented«. But OOP is just a means
to get things done, not a goal in itself.

Still, you can use classes with static elements to organize the
code and control visibilities of identifiers.

 

[mike3]

I'm going to assume that what you are asking is if it is OK to have
global functions. The answer is of course, even the standard library is
full of them. However, the best code is modular in nature, and the most
natural way to express a module in C++ is by using a class. I suggest
you read, or re-read, chapter 2 of "The C++ Programming Language" by
Stroustrup. He gives a good overview of the various programming
paradigms.

If you find yourself writing a bunch of switch statements, or if.. else
chains that all use the same condition, then you should probably
introduce more inheritance.

So then what, you have classes without data members then? It doesn't
make
sense as a data type, it's an algorithm after all, and there aren't
any
"global" variables that need to be tucked way, just functions.

Is it OK to have it contain lots of internal functions that are used
inside
it (e.g. the MakeTunnel stuff, etc.) but aren't for calling from
outside (the
only ones that'd be available to the outside would be the main
generator
routine which you punch in a level type to generate and the necessary
parameters
(including a map to use) and it generates that level on that map)?

So then if I had something like this:

class LevelGenerator {
private:
... (various functions like MakeTunnel, etc. all "static") ...
public:
static GenerateLevel(LevelMap&, LevelType); // Generates the level
};

is that bad? I guess the thing here is it doesn't seem very obvious at
all what the "best" way to factor it is, and if any of the approaches
really show any advantage. The main goal here is to minimize the
creation
of bugs in the future.

Or what if we had something like

class RoomMaker {
private:
... (exit point list, room coordinates, type) ...
public:
RoomMaker(u16, u16, u16, u16, RoomType); // parameters: coordinates,
type of room
~RoomMaker();
addDoorway(std::size_t); // add a doorway at one of
the room's exit points
dig(LevelMap&); // dig the room in a map
};

(maybe have an analogous "TunnelMaker" or something)

(and then the remaining stuff stays in that weird static-filled
"LevelGenerator" thing)

The latter is the approach I was taking to it, but was wondering
whether this was really a good idea or not.

 

[mike3]

Show me the implementation of your GenerateLevel function. If you want
faster turn around time than the newsgroup, you can email me
([email protected].)

Actually that routine itself is not yet written as of this posting. I
do
have a thing like the aforementioned RoomMaker, though. I'm just
wondering
about whether the overall organization, etc. is bad or not. Should one
have
a separate RoomMaker thing like that or not?

Though maybe you need the code to determine it (see the usage pattern,
etc.)...

 

[tonydee]

Tunnels and rooms are just patterns drawn on the map grid. The
routines I need will draw those patterns onto the grid. The
"MakeTunnel" and "MakeRoom" routines would just draw those
patterns to the map grid. The "MakeRoom" routine, for instance,
would need to take coordinates for the corners of the room's
bounding rectangle, which exit points should have doorways
stuck in them, and of course the type of room to draw on the
grid (which determines how many exit points there are
(for some room types, this number is proportional to the room's
size) and the shape of the room drawn inside the rectangle).

Well, it sounds like makeTunnel and makeRoom work on a particular map
grid object, so they're candidates for member functions thereof.
Stronger indications would be that you find that makes it easier - not
having to pass in and explicitly mention the map grid all the time, or
that you find yourself wanting some extra access to the map grid
that's not part of the public interface, and you wouldn't like to
expose. If those sort of benefits aren't significant, then separating
them (i.e. making makeTunnel and makeRoom freestanding functions) can
keep the map grid object interface and implementation cleaner and more
understandable.

Cheers,
Tony

 

[Stuart Redmann]

[snip]

So then what, you have classes without data members then?

Although this is most often a really strong indicator that the class
should is just a function, there are some (very esotheric) examples in
OOP where classes without data members are actually used. Google for
the State pattern. However, a class that has no data members _and_ no
virtual methods is _always_ a design error (unless you are using a
programming language that forces you to use classes).

It doesn't make
sense as a data type, it's an algorithm after all, and there aren't
any
"global" variables that need to be tucked way, just functions.

Then leave them as functions. Putting them as static functions into a
class has little advantage, except maybe that the class will then act
as a kind of namespace for the functions. If you want to achieve this,
you'd better use a real namespace so that your intention is clearer to
the reader of the code.

[snipped example code for creator of a labyrinth in a game]

I guess the thing here is it doesn't seem very obvious at
all what the "best" way to factor it is, and if any of the approaches
really show any advantage. The main goal here is to minimize the
creation
of bugs in the future.

I fully understand your concern, even if the following advice may
suggest otherwise: If you are not 100 percent sure which design to
choose (both seem to be equally good or bad), just choose one of them
arbitrarily (maybe tossing a coin) and start working.

If you feel ill at ease with your decision from the very first moment,
then you should try to write down what exacly bothers you. Most often
you'll be able to state your feelings of repulse only after you have
worked with your design for some time. If you reach this state, be
open to a complete re-design of what you have done so far.

I know that this sounds quite contrary to what you are taught in
software engineering lectures. My experience is that you often get a
much better understanding of the internal structure of your problem
when you work on it. This means that the design that you start with
may turn out to be the wrong one, but you wouldn't have come to this
realization unless you spent some time working with it.

Good luck,
Stuart

 

[mike3]

What about the LevelMap class? I'm wondering if "GenerateLevel" should
be in LevelMap instead of a separate function.

What code have you written so far?

LevelMap looks like this:

class LevelMap {
private:
....
public:
u16 getWidth() const
<inline>
u16 getHeight() const
<inline>
LevelTile& operator()(u16 y, u16 x)
<inline>
LevelTile operator()(u16 y, u16 x) const
<inline>

LevelMap();
LevelMap(u16, u16, int);
~LevelMap();
};

Not all that much, just wraps up a 2D grid of tiles (stored with
std::vector), with operators to access it like a matrix and get
the size (with guaranteed boundschecking enabled on debug builds
by using std::vector::at() in the operators for such builds).
Not yet implemented but will be at some point: save/load a map
to/from a disk file, load a predefined map template.

Even if I were to put GenerateLevel in LevelMap, what about the
whole RoomMaker thing?

 

[mike3]

[snip]

So then what, you have classes without data members then?

Although this is most often a really strong indicator that the class
should is just a function, there are some (very esotheric) examples in
OOP where classes without data members are actually used. Google for
the State pattern. However, a class that has no data members _and_ no
virtual methods is _always_ a design error (unless you are using a
programming language that forces you to use classes).

So then it'd be better to just bump that one down to a standalone
function?

Then leave them as functions. Putting them as static functions into a
class has little advantage, except maybe that the class will then act
as a kind of namespace for the functions. If you want to achieve this,
you'd better use a real namespace so that your intention is clearer to
the reader of the code.

That's what I thought of too.

[snipped example code for creator of a labyrinth in a game]

I guess the thing here is it doesn't seem very obvious at
all what the "best" way to factor it is, and if any of the approaches
really show any advantage. The main goal here is to minimize the
creation
of bugs in the future.

I fully understand your concern, even if the following advice may
suggest otherwise: If you are not 100 percent sure which design to
choose (both seem to be equally good or bad), just choose one of them
arbitrarily (maybe tossing a coin) and start working.

I suppose so. I could keep going with this approach and seeing where
it leads.

 

[mike3]

The LevelMap interface you show above is nothing more than a container
for LevelTiles. IMHO it doesn't even deserve mention in a design
diagram. Maybe you can show me the LevelTile class or one of the classes
that use this LevelMap class?

LevelTile looks like this:

class LevelTile {
private:
XCHAR intrinsicChar; /* Character intrinsic to this tile
*/
XCHAR displayedChar; /* Character displayed for this tile
*/
u8 flags; /* Flags (see above) */
u8 trapType; /* Type of trap on this tile */
u8 roomEffect; /* Permanent room effect */
u8 oneTimeEffect; /* One-time room effect */
u16 roomY, roomX; /* Coordinates of this tile in its
room
* (if part of a room).
*/
u8 moveFunc; /* Function triggered upon moving to
this
* tile.
*/
u8 aux1, aux2; /* Auxiliary data fields */
u8 buildaux; /* Auxiliary field used when
* building the level
*/

//std::vector<Item> items;
//std::auto_ptr<Monster> Monster;
public:
/* Inlined members */
XCHAR getIntrinsicChar() const
{ return(intrinsicChar); }

XCHAR getDisplayedChar() const
{ return(displayedChar); }

u8 getFlags() const
{ return(flags); }

u8 getTrapType() const
{ return(trapType); }

u8 getRoomEffect() const
{ return(roomEffect); }

u8 getOneTimeEffect() const
{ return(oneTimeEffect); }

u8 getRoomY() const
{ return(roomY); }

u8 getRoomX() const
{ return(roomX); }

u8 getMoveFunc() const
{ return(moveFunc); }

u8 getAux1() const
{ return(aux1); }

u8 getAux2() const
{ return(aux2); }

u8 getBuildAux() const
{ return(buildaux); }

void setTrapType(u8 trapType_)
{ trapType = trapType_; }

void clearOneTimeEffect()
{ oneTimeEffect = 0; }

void setMoveFunc(u8 moveFunc_)
{ moveFunc = moveFunc_; }

void setBuildAux(u8 buildaux_)
{ buildaux = buildaux_; }

/* Routines defined in leveltile.cpp */
LevelTile(); /* Default constructor */
LevelTile(const XCHAR&); /* Construct to specific intrinsic
char */
LevelTile(const XCHAR&, const XCHAR&); /* Construct with both
intrinsic
* and displayed chars specified
*/
LevelTile(const XCHAR&, u16, u16); /* Construct to specific
intrinsic char
* and room y/x coordinates
*/
LevelTile(const XCHAR&, const XCHAR&,
u16, u16); /* Construct to specific intrinsic/
displayed
* chars and room y/x coordinates
*/
LevelTile(const XCHAR&, const XCHAR&,
u8, u8, u8,
u8, u16, u16, u8,
u8, u8, u8); /* Construct with all fields specified */
~LevelTile(); /* Destructor */
};

It just holds various data fields. Due to that, I've wondered
whether it'd be better to just eliminate all the accessors and
just making it a "struct" instead (or "public") w/constructors
to fill in default values for various prefabbed tile types. Is
that a good idea or not? I've heard this stuff about "don't use
structs", etc. (which is the only reason it's the way it is) but
in this case I can't really think of anything else this should be.
Except there's going to be routines at some point that open/close
doors, handle trap triggering, etc. and I'm not sure if those
could/should go in there or not.

 

[mike3]

LevelTile looks like this:

[snipped wrapped struct]

It just holds various data fields. Due to that, I've wondered whether
it'd be better to just eliminate all the accessors and just making it
a "struct" instead (or "public") w/constructors to fill in default
values for various prefabbed tile types. Is that a good idea or not?

It's likely a bad idea to turn LevelTile into a struct. With what you
have, at least you can move toward a better design. Let me see a
function or two that uses a LevelTile object.

OK, I'll leave it a class then. As for functions that use them,
currently
it's just the level generator, where I've got two routines that invoke
one of the constructors with a set of default parameters for making
wall/
floor tiles. (room floor tiles, for various reasons, have a flag
TFL_ROOM
set, thus this routine provides a way to wrap that setting and
others.)

I'm especially interested in the 'moveFunc' member variable, it
obviously isn't a function even though the comments describe it as
such...

That's because there are actually two functions, not one, that will
be involved. (one for the player, one for monsters.) This is a
not-yet-implemented feature. "moveFunc" holds a code (will likely
be from an enum, in which case I'll switch the type to that of the
enum -- I just don't have it as one yet since that feature is not
yet implemented) that will reference the function to be called.
When implemented, I'll likely add some member to LevelTile to
do this. Storing a function pointer would not work here -- for one,
it's not as easy to save that out to a file (you can't just save
a pointer that won't necessarily be the same on the next run, you'd
have to compare, and that sounds pretty dorky and bad.).

'trapType' should probably be a Trap class instead of a integer as well.
How is trapType being used?

Again, this is another to-be-added feature. It stores which type of
trap, if any, should be on the tile. When the player or a monster
moves
onto the tile, this would activate. I suppose I could make a small
"Trap" class wrapping this field plus a pair of functions to handle
moves by player or monster. If not, it'll probably become another
enum too (like moveFunc).

Would they modify any of the states of a LevelTile? Have you written any
of these routines yet?

Yes and no, respectively.

 

[mike3]

mike3 <[email protected]> wrote:

So your plan is to have a few switch statements that all use "moveFunc"
to decide what to do. This begs for inheritance (if "moveFunc" never
changes once set,) or the strategy pattern (if "moveFunc" can change.)

Inheritance of what from what?

Also, you say that the value of moveFunc depends on what type of
creature is in the room, player or monster? Those should probably be
classes...

The _value_ does not depend, but which function is called does. And
not
"which type is in the room" but "which type moves over the tile". It
is
triggered on moving over the tile.

class MobileObject {
public:
   virtual ~MobileObject() { }
   virtual void handleMove(unsigned char moveFunc) = 0;

};

class Monster : public MobileObject {
public:
   virtual void handleMove(unsigned char moveFunc);

};

class Player : public MobileObject {
public:
   virtual void handleMove(unsigned char moveFunc);

};

So then when the object moves onto the tile, we pass
the tile's moveFunc value through the handleMove function?
Shouldn't that be done internally, inside whatever
function moves the mover from one tile to another?
Otherwise we could neglect that call.

Presumably each trap type does something different? I suggest that you
go ahead and make a Trap class. Give it a setter, but no getter. So
something like this:

class Trap {
   unsigned char trapType;
public:
   Trap(unsigned char trapType): trapType(trapType) { }
   void setTrapType(unsigned char v) { trapType = v; }

};

I was thinking of something like that, with an additional routine to
trigger (use) the trap.

When you find yourself wanting to use the value from outside the class,
don't. Instead make a function in the class that does the right thing.

The above is a good general rule to start with. Remove your getters and
that will tend to force you into a better object oriented design. (Don't
get too pedantic about this idea though, sometimes a getter is just what
you need.)

Maybe I should remove all the getters and only add ones back when
really
necessary (like, e.g. the buildaux field, which the level generator
uses
to store information while generating the level)?

Any proposed function that modifies the state of a LevelTile should be a
member-function of the LevelTile class.

That's what I thought.

 

[mike3]

If you were to list all your moveFunc values in an enum, what would you
call the enum? What would the individual values be?

Probably something like "MoveFunc".

When you have code like:

enum Foo { A, B, C };

There's a good chance you can do:

class Foo { };

class A : public Foo { };
class B : public Foo { };
class C : public Foo { };

But the different MoveFunc are assigned on a per-tile basis. So
the above would implying having different types of tiles, i.e.
a bunch of:

class LevelTile { ... }

class LevelTileWithSign1Move : public LevelTile { ... }
class LevelTileWithSign2Move : public LevelTile { ... }

....

Is it OK to throw all these into a vector of LevelTile (like the
LevelMap)? It would seem here you'd need to make LevelMap have
a vector of pointers, then. Is that a good idea?

And why should we stop at moveFunc? But then combinatorially,
things get really big. In addition, if I want to save
the map to disk (something I will want to do!), we need to make
sure the type of moveFunc needs to get saved out, and then when
we load in, we need to have a switch() somewhere to switch between
the different kinds of LevelTile and, see where this is going?
(What if I add a new moveFunc but forgot about that switch()?
Boom. Bug.) It just seems to be getting more complicated, and I
don't see a huge advantage here. Gains in complexity with no
clear or significant advantages are not a good idea, are they?

The question is, would you even need to pass in "moveFunc"?

You listed it as a parameter going into handleMove in the
player and monster class. I wasn't clear on the intention
behind that. And now that you said that, I'm even less clear.

I suggest you try it and see what happens. You don't have to actually
remove the getters of course, you could simply resist using them.

However, unused stuff seems like clutter, so why not just
cut it out?