• Announcements

    • khawk

      Download the Game Design and Indie Game Marketing Freebook   07/19/17

      GameDev.net and CRC Press have teamed up to bring a free ebook of content curated from top titles published by CRC Press. The freebook, Practices of Game Design & Indie Game Marketing, includes chapters from The Art of Game Design: A Book of Lenses, A Practical Guide to Indie Game Marketing, and An Architectural Approach to Level Design. The GameDev.net FreeBook is relevant to game designers, developers, and those interested in learning more about the challenges in game development. We know game development can be a tough discipline and business, so we picked several chapters from CRC Press titles that we thought would be of interest to you, the GameDev.net audience, in your journey to design, develop, and market your next game. The free ebook is available through CRC Press by clicking here. The Curated Books The Art of Game Design: A Book of Lenses, Second Edition, by Jesse Schell Presents 100+ sets of questions, or different lenses, for viewing a game’s design, encompassing diverse fields such as psychology, architecture, music, film, software engineering, theme park design, mathematics, anthropology, and more. Written by one of the world's top game designers, this book describes the deepest and most fundamental principles of game design, demonstrating how tactics used in board, card, and athletic games also work in video games. It provides practical instruction on creating world-class games that will be played again and again. View it here. A Practical Guide to Indie Game Marketing, by Joel Dreskin Marketing is an essential but too frequently overlooked or minimized component of the release plan for indie games. A Practical Guide to Indie Game Marketing provides you with the tools needed to build visibility and sell your indie games. With special focus on those developers with small budgets and limited staff and resources, this book is packed with tangible recommendations and techniques that you can put to use immediately. As a seasoned professional of the indie game arena, author Joel Dreskin gives you insight into practical, real-world experiences of marketing numerous successful games and also provides stories of the failures. View it here. An Architectural Approach to Level Design This is one of the first books to integrate architectural and spatial design theory with the field of level design. The book presents architectural techniques and theories for level designers to use in their own work. It connects architecture and level design in different ways that address the practical elements of how designers construct space and the experiential elements of how and why humans interact with this space. Throughout the text, readers learn skills for spatial layout, evoking emotion through gamespaces, and creating better levels through architectural theory. View it here. Learn more and download the ebook by clicking here. Did you know? GameDev.net and CRC Press also recently teamed up to bring GDNet+ Members up to a 20% discount on all CRC Press books. Learn more about this and other benefits here.
Sign in to follow this  
Followers 0
fastcall22

My ever-evolving coding style

14 posts in this topic

void frobnicate()
{
    CFoobar *foo = new CFoobar();
    
    //Fixed
    int* var = foo->GetFoo();
    *var += 7;

    double thing = *foo->GetBaz();
    // etc...
    
    delete foo;
}

 

Honestly I like returning pointers for most cases. Using the pointer you dont have to make a thousand function calls. Especially in parallel programming this is very useful when you want to avoid static variables. Sometimes its not a good idea, or not even possible depending on the use, but I believe this is a perfectly valid programming habit. IMO

Edited by Slig Commando
1

Share this post


Link to post
Share on other sites

Honestly I like returning pointers for most cases. Using the pointer you dont have to make a thousand function calls. Especially in parallel programming this is very useful when you want to avoid static variables. Sometimes its not a good idea, or not even possible depending on the use, but I believe this is a perfectly valid programming habit. IMO

You do realize that that method destroys your ability to enforce class invariants? And that it has no advantage over simply making your members public (and has the disadvantage of being clunky and a really non-obvious and non-idiomatic way of modifying an object)?And that this doesn't do anything to help avoid static variables (how is this even related to static variables?)?

 

There might be a few exceptional cases where it's a decent idea, but it should be just that, exceptional cases. If it's your preferred/common method... something is wrong.

Edited by Cornstalks
1

Share this post


Link to post
Share on other sites

Honestly I like returning pointers for most cases

I think the point is that his 'getters' are useless, because his code is equivalent to as if the variables were just public...

It's just a really bloated way of writing this:

struct Foobar
{
    int foo;
    RGB bar;
    double baz;
};
void frobnicate()
{
    Foobar foo;
    foo->foo = foo->foo+7;
    double thing = foo->baz;
}
2

Share this post


Link to post
Share on other sites

Whenever writing a million getters and setters gets annoying, I usually ask myself two things:

 

a) if I just go and write trivial set/get methods for everything, why not just stop pretending and make it public?

 

b) why not be lazy and wrap it in a template that handles it?

 

class ...

{

...

   Property<int> stuff;

};

 

int value = obj.stuff();

obj.stuff(5);

obj.stuff( obj.stuff() + 5 );

 

Not calling it get and set allows to use the template for all trivial cases and do custom implementations where needed without requiring a different syntax.

 

Still, if a class has a dozen members and trivial accessors for all of them, encapsulation probably just went out the window. So it's more of a convenient way to remain consistent.

1

Share this post


Link to post
Share on other sites

Yes, if your class has a million getters and setters it's probably doing far too much for its own good. I like the template idea, it reduces boilerplate code while preserving encapsulation. On the other hand, making the member public is not a good idea unless it's a trivial member or a trivial class, because as soon as you need to enforce a condition on this member your class interface has to change, and all code depending on the class breaks (sure, the fix is a find/replace away, but then you need to recompile etc..)

 

I kind of liked the Object Pascal/Delphi way for properties (sorry if there are any syntax errors, it's been months since I wrote any):

 

interface

...

private
  fFoo: Integer;
  fBar: String;

public
  property Foo: Integer read fFoo; // read-only, default getter (no boilerplate getter method)
  property Bar: String read getBar write setBar; // read-write, custom getter/setters

...

implementation

function getBar: String;
begin
  if (fBar <> "sentinel") then Result := fBar;
end;

procedure setBar(Value: String);
begin
  if (Value <> "sentinel") then fBar := Value;
end;

Which I think is advantageous in that you immediately see from the interface what is read-only and what is read-write (or write-only, should you have a use case for that) and you can easily switch from a default getter (or setter) without even changing the class interface. As far as client code goes, it just thinks Foo and Bar are properties as if they were ordinary members (accessed by class.Foo) and any reads go through the getter, any writes go through the setter. It also works with indexed properties. I think it's rather neat, I'm sure it has its downsides though.

 

Of course, if you are absolutely certain that any value of the member is valid and you don't need to do anything when it changes, and that you will never reconsider this design decision, just make it public. This is rather rare, though, usually this kind of member is set inside a constructor or other initialization method via some argument.

0

Share this post


Link to post
Share on other sites

Today, my subconcious decided to be helpful and torture me with randomly fetched memories of my old coding style

I wonder what you're trying to tell yourself by self-inflicting this torture. Thanks god those things are gone! tongue.png
0

Share this post


Link to post
Share on other sites

Oh god, when I decided to check my old Battle City code (from about 9 months ago) and found stuff like this:

 

namespace BattleTanks
{
    class Tank
    {
        #region Variables
        public void LoadContent(ContentManager content, string TankAssetName, string BulletAssetName) { ... }
        public void Draw(SpriteBatch spriteBatch) { ... }
        public void Initialize(int X, int Y, float rotation, float scaling, int speed, bool playerbullet) { ... }
        public void MoveTank(bool HorAxis, int direction, bool cancel) { ... }
        public void RemoveTank(TimeSpan time) { ... }
        public void MoveBullet() { ... }
        public void LaunchBullet() { ... }
        public string BulletDirection() { ... }
        public void RemoveBullet() { ... }
        public void RotateAI(out bool HorAxis, out int direction, out string text) { ... }
        public void FireAI() { ... }
        public void TurnAI(string direction) { ... }
    }
}

And I was happy at that time, especially because it worked. Try to guess what is hidden inside some of the methods.

 

2 months later I open the same code out of curiosity... Reaction: "What in the mother of f- was I doing back then >.<"

 

And for some reason I'm pretty sure that I'll say the same for my current code after 1 year.

0

Share this post


Link to post
Share on other sites

Try to guess what is hidden inside some of the methods.

  • LoadContent: retrieves the assets used by this tank and stores them in the member variables.
  • Draw: adds the tank sprite to the sprite batch so it gets drawn later.
  • Initialize: sets some member variables that affect the behavior of the tank.
  • MoveTank: makes the tank move in the specific direction, applying physics as needed.
  • RemoveTank: called when a tank is exploding? No idea what the time is for.
  • MoveBullet: as MoveTank but for the bullets of each tank. I guess they're tied to tanks to ensure they can shoot only one bullet each (doesn't seem very smart, a tank could blow up before the bullet vanishes).
  • LaunchBullet: makes the tank shoot a bullet.
  • BulletDirection: retrieve bullet's direction? Why is this a string?! o_O
  • RemoveBullet: gets rid of the bullet when it hits something.
  • RotateAI: makes the AI try to rotate in a specific direction. Again, what is the string for?!
  • FireAI: makes the AI try to fire a bullet.
  • TurnAI: Like RotateAI but giving the angle directly. ...also I take that, the angles are given as strings -_-'

How much of that was wrong?

0

Share this post


Link to post
Share on other sites

BulletDirection: retrieve bullet's direction? Why is this a string?! o_O

 

Hey, it's just flexible design. That way the function can return, in addition to actual directions, "over there", "this way", "behind you" or even "nowhere". You can never have too many strings.

1

Share this post


Link to post
Share on other sites

Try to guess what is hidden inside some of the methods.

  • LoadContent: retrieves the assets used by this tank and stores them in the member variables.
  • Draw: adds the tank sprite to the sprite batch so it gets drawn later.
  • Initialize: sets some member variables that affect the behavior of the tank. Constructor? What's that?
  • MoveTank: makes the tank move in the specific direction, applying physics as needed.
  • RemoveTank: called when a tank is exploding? No idea what the time is for. Respawn timing.
  • MoveBullet: as MoveTank but for the bullets of each tank. I guess they're tied to tanks to ensure they can shoot only one bullet each (doesn't seem very smart, a tank could blow up before the bullet vanishes). Problem solved long ago by splitting the damn superclass into 3 normal ones.
  • LaunchBullet: makes the tank shoot a bullet.
  • BulletDirection: retrieve bullet's direction? Why is this a string?! o_O
  • RemoveBullet: gets rid of the bullet when it hits something.
  • RotateAI: makes the AI try to rotate in a specific direction. Again, what is the string for?!
  • FireAI: makes the AI try to fire a bullet.
  • TurnAI: Like RotateAI but giving the angle directly. ...also I take that, the angles are given as strings sleep.png'

How much of that was wrong?

 

Comments added in quote with blue font.

 

Regarding angles and strings - it was for convenience; movement is restricted to 4 angles (north, south, west, east) so it's easier to perform direction checks with strings instead of writing Math.PI * (0, 0.5, 1, 1.5).

 

Don't worry, I'm using constants now. Not to mention that I splitted "TankBulletAI" class into 3 separate ones, each one tracking only what it's SUPPOSED to track.

 

Oh yeah, everything was also stored in arrays instead of lists. And I was wondering why did game crash when only 2 enemies remained (out of 30) in order to finish the level...

0

Share this post


Link to post
Share on other sites

Honestly I like returning pointers for most cases. Using the pointer you dont have to make a thousand function calls. Especially in parallel programming this is very useful when you want to avoid static variables. Sometimes its not a good idea, or not even possible depending on the use, but I believe this is a perfectly valid programming habit. IMO

You do realize that that method destroys your ability to enforce class invariants? And that it has no advantage over simply making your members public (and has the disadvantage of being clunky and a really non-obvious and non-idiomatic way of modifying an object)?And that this doesn't do anything to help avoid static variables (how is this even related to static variables?)?

 

There might be a few exceptional cases where it's a decent idea, but it should be just that, exceptional cases. If it's your preferred/common method... something is wrong.

 

It is all dependent on what it is you are programming and what the expectations of that program are. To say their are only a few exceptional cases where you would return a pointer and that it has no advantages is purely based on ones own perspective.

 

My current, long term project is a game engine I have been working on for some time now. The overall goal is parallelism. Being able to pass off a single object pointer to multiple subsystems so that each can do its work at the same time, to me, is an intelligent design. Whether it is the object itself, or maybe just the draw location of a game entity being passed to the physics engine and to the player for keyboard input(gravity must be applied, force from other physics objects may be applied, and the player keyboard input all effect the final draw location of an in-game object). Of course their are cases where placing locks on certain members of that object are necessary, but the overall advantage I feel outweighs any syntax or non-obvious issues that may be encountered.

0

Share this post


Link to post
Share on other sites

Respawn timing.

...suddenly I feel like an idiot for not realizing that one (sorta dumb though when you consider it's probably the same time always).

 

Regarding angles and strings - it was for convenience; movement is restricted to 4 angles (north, south, west, east) so it's easier to perform direction checks with strings instead of writing Math.PI * (0, 0.5, 1, 1.5).

I would have just used 0, 1, 2, 3 for something like that (or better yet, an enum, but the basic principle is the same). Bonus because checking integers is much easier and faster than the alternatives.

1

Share this post


Link to post
Share on other sites

Regarding angles and strings - it was for convenience; movement is restricted to 4 angles (north, south, west, east) so it's easier to perform direction checks with strings instead of writing Math.PI * (0, 0.5, 1, 1.5).

I would have just used 0, 1, 2, 3 for something like that (or better yet, an enum, but the basic principle is the same). Bonus because checking integers is much easier and faster than the alternatives.

Yep. Actually I use two of them, eDir2d4 and eDir2d8... just make sure you keep track of which one your variable is. And even when I do need real angles, I prefer a 16-bit value instead of float/double in radians. 0x10000 is one full rotation, so it overflows and wraps back to 0 automatically. And you can reinterpret as signed 16 bit to get a -180 to +180 degree angle for situations like figuring whether it's quicker to turn left or right to point yourself toward something. And a quick bitshift of the unsigned 16 bit value converts to and from the eDir2d enums smile.png And you can use a 32-bit number if you do need to go over 360 degrees (e.g. a loop counter going from 0 to 360). Really convenient.

 

As for the original post of get/set madness, I agree with Hodgman that if you're giving out pointers to private variables, they should just be public. I have been known to write lots of get/set pairs though. Gets are generally "pure", just return a value or const reference to the variable. But sets sometimes do extra. And a lot of the time they're actually just pass-throughs to get/set functions of a member of the class. For example, in my 2D tiled game map editor, the MainWindow class has this, which I'm undecided whether it's a nightmare or not:

[source]    bool isMapListWindowVisible() const { return mMapListWindow->isVisible(); }
    bool isTilesetWindowVisible() const { return mTilesetWindow->isVisible(); }
    bool isSpriteWindowVisible() const { return mSpriteWindow->isVisible(); }
    bool isLayerWindowVisible() const { return mLayerWindow->isVisible(); }
    bool isGameViewVisible() const { return mMapEditorWindow->isGameViewVisible(); }
    bool getSpriteMarkersVisible() const { return mMapEditorWindow->getSpriteMarkersVisible(); }
    bool getFrontLayersTransparent() const { return mMapEditorWindow->getFrontLayersTransparent(); }
    void setMapListWindowVisible(bool visible) { mMapListWindow->setVisible(visible); resizeRedraw(); }
    void setTilesetWindowVisible(bool visible) { mSpriteWindow->setVisible(false); mTilesetWindow->setVisible(visible); resizeRedraw(); }
    void setSpriteWindowVisible(bool visible) { mTilesetWindow->setVisible(false); mSpriteWindow->setVisible(visible); resizeRedraw(); }
    void setLayerWindowVisible(bool visible) { mLayerWindow->setVisible(visible); resizeRedraw(); }
    void setGameViewVisible(bool visible) { mMapEditorWindow->setGameViewVisible(visible); }
    void setSpriteMarkersVisible(bool visible) { mMapEditorWindow->setSpriteMarkersVisible(visible); }
    void setFrontLayersTransparent(bool transparent) { mMapEditorWindow->setFrontLayersTransparent(transparent); }
[/source]

They're mostly there so the menu bar of the main window can call them when you switch on and off the visibility of all the things. And it does eliminate the need for the main window to give access to its sub-windows. But it's quite a wall of text.

0

Share this post


Link to post
Share on other sites

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  
Followers 0