Jump to content
  • Advertisement
Sign in to follow this  
Roots

Using a global declarations header file: good idea or bad idea?

This topic is 1981 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

A practice that my project has been doing for a long time is to maintain a header file that lists declarations of nearly every single class in their respective namespaces. We did this because back when our code was much more volatile, we'd end up with numerous class declarations at the start of header files, some of which were later renamed or were no longer used/needed. Thus the code became a bit of a mess, and someone reading the code would get mistaken assumptions when reading the list of class declarations. We also had some problems with recursive file inclusion before this solution was put in place.

 

So instead, now nearly every header file in our source tree includes this one, so all class and other necessary declarations are made available. The primary disadvantage to this is that whenever you make any changes to this file, nearly the entire project has to be re-compiled and linked, which can take a long time for a large project like ours (over 100K lines of code). However, our code base is mature enough now that changes to this file are infrequent so it's not a huge issue.

 

What do you think of this practice? Is this something that you do in your own projects, or do you have an alternative? One guy who forked our game and created his own was insistent about removing this file because he didn't like the long compile times when he made changes to it (and AFAIK, that was his only reason). So I wanted to see what others thought.

 

Below is the file in question, for your leisure.

 

[source]

///////////////////////////////////////////////////////////////////////////////
//            Copyright (C) 2004-2013 by The Allacrost Project
//                         All Rights Reserved
//
// This code is licensed under the GNU GPL version 2. It is free software
// and you may modify it and/or redistribute it under the terms of this license.
// See http://www.gnu.org/copyleft/gpl.html for details.
///////////////////////////////////////////////////////////////////////////////

/** ***************************************************************************
*** \file    defs.h
*** \author  Tyler Olsen, roots@allacrost.org
*** \brief   Header file for forward declarations of classes and debug variables.
***
*** This file serves two purposes. The first purpose of this file is to forward
*** declare classes and shared variables in order to avoid problems with
*** recursive inclusion. The second purpose of this file is to declare
*** the functions that contains all of the Lua binding code. This makes the C++
*** C++ code available for use in Lua scripts.
***
*** \note Pretty much every header file in the source tree will need to include
*** this file, with a few exceptions (utils.h is one). The only source file
*** that should need to include this file is defs.cpp
***
*** \note The commenting for all namespaces, variables, and classes declared in
*** this file can be found in the respective header files for where these
*** structures reside in. There are no doxygen comments for the classes and
*** namespaces found here.
***
*** \note You should not need to use the hoa_defs namespace unless you are
*** making the call to bind the engine to Lua.
*** **************************************************************************/

#ifndef __DEFS_HEADER__
#define __DEFS_HEADER__

////////////////////////////////////////////////////////////////////////////////
// Game Engine Declarations
////////////////////////////////////////////////////////////////////////////////

// Audio declarations, see src/engine/audio/
namespace hoa_audio {
    extern bool AUDIO_DEBUG;
    class AudioEngine;

    class AudioDescriptor;
    class MusicDescriptor;
    class SoundDescriptor;

    namespace private_audio {
        class AudioCacheElement;

        class AudioBuffer;
        class AudioSource;
        class AudioStream;

        class AudioInput;
        class WavFile;
        class OggFile;
        class AudioMemory;

        class AudioEffect;
        class FadeInEffect;
        class FadeOutEffect;
    }
}

// Video declarations, see src/engine/video/
namespace hoa_video {
    extern bool VIDEO_DEBUG;
    class VideoEngine;

    class Color;
    class CoordSys;
    class ScreenRect;

    class FixedImageNode;
    class VariableImageNode;

    class ImageDescriptor;
    class StillImage;
    class AnimatedImage;
    class CompositeImage;

    class TextureController;

    class TextSupervisor;
    class FontGlyph;
    class FontProperties;
    class TextImage;

    class Interpolator;

    class ParticleEffect;
    class ParticleEffectDef;
    class ParticleEmitter;
    class EffectParameters;

    namespace private_video {
        class Context;

        class TexSheet;
        class FixedTexSheet;
        class VariableTexSheet;
        class FixedTexNode;
        class VariableTexNode;

        class ImageMemory;

        class BaseTexture;
        class ImageTexture;
        class TextTexture;
        class TextElement;
        class AnimationFrame;
        class ImageElement;

        class ParticleManager;
        class ParticleSystem;
        class ParticleSystemDef;
        class Particle;
        class ParticleVertex;
        class ParticleTexCoord;
        class ParticleKeyframe;

        class ScreenFader;
        class ShakeForce;
    }
}

// Script declarations, see src/engine/script/
namespace hoa_script {
    extern bool SCRIPT_DEBUG;
    class ScriptEngine;

    class ScriptDescriptor;
    class ReadScriptDescriptor;
    class WriteScriptDescriptor;
    class ModifyScriptDescriptor;
}

// Mode manager declarations, see src/engine/
namespace hoa_mode_manager {
    extern bool MODE_MANAGER_DEBUG;
    class ModeEngine;

    class GameMode;
}

// Input declarations, see src/engine/
namespace hoa_input {
    extern bool INPUT_DEBUG;
    class InputEngine;
}

// Settings declarations, see src/engine/
namespace hoa_system {
    extern bool SYSTEM_DEBUG;
    class SystemEngine;
    class Timer;
}

////////////////////////////////////////////////////////////////////////////////
// Common Code Declarations
////////////////////////////////////////////////////////////////////////////////

// Common declarations, see src/common
namespace hoa_common {
    extern bool COMMON_DEBUG;
    class CommonDialogue;
    class CommonDialogueOptions;
    class CommonDialogueWindow;
    class CommonDialogueSupervisor;
}

// Global declarations, see src/common/global/
namespace hoa_global {
    extern bool GLOBAL_DEBUG;
    class GameGlobal;
    class GlobalEventGroup;

    class GlobalObject;
    class GlobalItem;
    class GlobalWeapon;
    class GlobalArmor;
    class GlobalShard;
    class GlobalKeyItem;

    class GlobalStatusEffect;
    class GlobalElementalEffect;
    class GlobalSkill;

    class GlobalAttackPoint;
    class GlobalActor;
    class GlobalCharacter;
    class GlobalEnemy;
    class GlobalParty;
}

// GUI declarations, see src/common/gui
namespace hoa_gui {
    class GUISystem;
    class MenuWindow;
    class TextBox;
    class OptionBox;

    namespace private_gui {
        class GUIElement;
        class GUIControl;
        class MenuSkin;

        class Option;
        class OptionElement;
        class OptionCellBounds;
    }
}

////////////////////////////////////////////////////////////////////////////////
// Game Mode Declarations
////////////////////////////////////////////////////////////////////////////////

// Battle mode declarations, see src/modes/battle/
namespace hoa_battle {
    extern bool BATTLE_DEBUG;
    class BattleMode;

    namespace private_battle {
        class BattleMedia;

        class SequenceSupervisor;

        class BattleActor;
        class BattleCharacter;
        class BattleEnemy;

        class BattleAction;
        class SkillAction;
        class ItemAction;

        class BattleTimer;
        class BattleTarget;
        class BattleItem;

        class BattleSpeaker;
        class BattleDialogue;
        class DialogueSupervisor;

        class BattleStatusEffect;
        class EffectsSupervisor;

        class IndicatorElement;
        class IndicatorText;
        class IndicatorImage;
        class IndicatorSupervisor;

        class ItemCommand;
        class SkillCommand;
        class CharacterCommand;
        class CommandSupervisor;

        class CharacterGrowth;
        class FinishDefeatAssistant;
        class FinishVictoryAssistant;
        class FinishSupervisor;
    }
}

// Boot mode declarations, see src/modes/boot/
namespace hoa_boot {
    extern bool BOOT_DEBUG;
    class BootMode;

    namespace private_boot {
        class BootMenu;
        class CreditsWindow;
        class WelcomeWindow;
    }
}

// Map mode declarations, see src/modes/map/
namespace hoa_map {
    extern bool MAP_DEBUG;
    class MapMode;

    namespace private_map {
        class TileSupervisor;
        class MapTile;

        class MapRectangle;
        class MapFrame;
        class PathNode;

        class ObjectSupervisor;
        class MapObject;
        class PhysicalObject;
        class TreasureObject;

        class VirtualSprite;
        class MapSprite;
        class EnemySprite;

        class DialogueSupervisor;
        class SpriteDialogue;
        class MapDialogueOptions;

        class EventSupervisor;
        class MapEvent;
        class DialogueEvent;
        class ScriptedEvent;
        class ShopEvent;
        class SoundEvent;
        class MapTransitionEvent;
        class JoinPartyEvent;
        class BattleEncounterEvent;
        class SpriteEvent;
        class ScriptedSpriteEvent;
        class ChangeDirectionSpriteEvent;
        class PathMoveSpriteEvent;
        class RandomMoveSpriteEvent;
        class AnimateSpriteEvent;

        class TreasureSupervisor;
        class MapTreasure;

        class ZoneSection;
        class MapZone;
        class ResidentZone;
        class EnemyZone;
        class ContextZone;
    }
}

// Menu mode declarations, see src/modes/menu/
namespace hoa_menu {
    extern bool MENU_DEBUG;
    class MenuMode;
}

// Pause mode declarations, see src/modes/
namespace hoa_pause {
    extern bool PAUSE_DEBUG;
    class PauseMode;
}

// Scene mode declarations, see src/modes/
namespace hoa_scene {
    extern bool SCENE_DEBUG;
    class SceneMode;
}

// Shop mode declarations, see src/modes/shop/
namespace hoa_shop {
    extern bool SHOP_DEBUG;
    class ShopMode;

    namespace private_shop {
        class ShopMedia;
        class ShopInterface;
        class ShopObject;
        class ShopObjectViewer;
        class ObjectListDisplay;

        class RootInterface;
        class CategoryDrawData;

        class BuyInterface;
        class BuyListDisplay;

        class SellInterface;
        class SellListDisplay;

        class TradeInterface;

        class ConfirmInterface;

        class LeaveInterface;
    }
}

// Test mode declarations, see src/modes/
namespace hoa_test {
    extern bool TEST_DEBUG;
    class TestMode;
}

////////////////////////////////////////////////////////////////////////////////
// Miscellaneous Declarations
////////////////////////////////////////////////////////////////////////////////

// Utils declarations, see src/utils.h
namespace hoa_utils {
    extern bool UTILS_DEBUG;
    class ustring;
    class Exception;
    extern float RandomFloat();
}

////////////////////////////////////////////////////////////////////////////////
// Binding Declarations
////////////////////////////////////////////////////////////////////////////////

//! \brief Namespace which contains all binding functions
namespace hoa_defs {

/** \brief Contains the binding code which makes the C++ engine available to Lua
*** This method should <b>only be called once</b>. It must be called after the
*** ScriptEngine is initialized, otherwise the application will crash.
**/
void BindEngineCode();
void BindCommonCode();
void BindModeCode();

} // namespace hoa_defs

#endif // __DEFS_HEADER__

[/source]

Share this post


Link to post
Share on other sites
Advertisement

This is a code smell to me. You say that "nearly every header file in our source tree includes this one". That means that every system knows about every other system. In practice, only you can say what the negative effects are day to day, but I'd ask what are the benefits of this approach? 

 

Instead of this header, the normal C++ idiom is to forward declare just what you need in a class header.

Share this post


Link to post
Share on other sites

They only know the names of the other systems, and that's it. It's not as if every file is including our audio engine even if they don't use any audio. They still have to include the appropriate header files that they use, (#include "audio.h" for example).

 

The primary benefit is, as I said, you don't end up with a sloppy and perpetually outdated list of declarations across your code. I realize that it's standard to only declare what you need in a header, but it's far too easy to remove some code that you used to use and forget to remove the declaration that it required. Or when a class gets renamed, adding in its new declaration and forgetting to remove the old one. The primary benefit is keeping code clean, as I said. I'm rather OCD about keeping my code organized and easy to read (both for myself and for the sake of others looking to understand it). Sure it's not a huge benefit and we could easily do without it, but keeping class declarations updated in one file is a heck of a lot easier than keeping them updated across numerous files.

Share this post


Link to post
Share on other sites

To me "clean" code is code that only includes or forward declares the bare minimum it needs to.  It also gives me some information about what is being used in that file, or files that include that file.  I dont mind a few forward declarations being there, that's just the nature of the language.

 

So, I'd frown on this practice.  I agree with the previous post that says it smells.  I try to keep my global include files to only include stuff that is actually intended to be global.

Share this post


Link to post
Share on other sites

I understand your argument and I agree with you. Ideally, I'd like to only forward declare what is necessary. We still receive some information about what is used in files based upon what header files are included by its corresponding source file (almost every header file has an accompanying source file in our project). This would further help if you structure your class/file organization to resemble Java, where one and only one class is allowed to be defined in any file.

 

 

So I do understand and appreciate the arguments made thus far against this practice. I still enjoy this practice it because it makes it so much easier to just #include "defs.h" and then not have to worry about declaration lists any further. I guess it's caused me to become a little bit lazy. rolleyes.gif

Share this post


Link to post
Share on other sites
One alternative if your forward declarations get complex or if you have other concerns about keeping them in sync is to create forward declaration headers for specific headers. So you'd have a foo.h header which contains class definitions but a foo_fwd.h header that contains just the forward declarations for the classes in foo.h.

Share this post


Link to post
Share on other sites
They only know the names of the other systems, and that's it.

And that is too much for modular design.
Why would the graphics library even need to know just the names or anything at all about the sound library?

I still enjoy this practice it because it makes it so much easier to just #include "defs.h" and then not have to worry about declaration lists any further.

Deciding on best practices is, you will soon find out, based on what makes your debugging and maintenance easier, not what makes your code just slightly easier to write here and now.

 

We are giving you advice based on a heap-load of experience—too much to explain all the details.

Some are easy to point out, such as long compile times and logical fallacies.

 

Some you wouldn’t even appreciate even if we pointed them out just because you don’t have enough experience maintaining huge and complex systems.

And by that time you will find out the hard way how obviously better the alternatives make your long-run life.

 

 

I personally feel that your proposal reflects poorly on your programmer mentality, at least depending on your level.

Beginners have an excuse, but if you are experienced, bad dog.  This says to me you knowingly ignore modular concepts in favor of a few saved keystrokes and some saved mental effort.  I wouldn’t hire this kind of laziness/recklessness.  It is the programming equivalent of drunk driving.

If you are intermediate, what bothers me most is that you actually tried to justify it by “but it’s easy”.  Intermediates might not know the right path but they should know enough about the wrong path to never justify bad practice with “it’s easy”.  A proper intermediate starts down to right road when that road is pointed out or discovered.  No excuses.

It tells me you don’t really care too much about your future and seem to have little motivation for programming.  Again undesirable traits in a recruit.

 

I am sure you are completely different from that, right?  You have what it takes, right?

I am glad you know it, but now you should start to show it.

 

 

L. Spiro

Share this post


Link to post
Share on other sites
I understand your argument and I agree with you. Ideally, I'd like to only forward declare what is necessary. We still receive some information about what is used in files based upon what header files are included by its corresponding source file (almost every header file has an accompanying source file in our project). This would further help if you structure your class/file organization to resemble Java, where one and only one class is allowed to be defined in any file.

 

 

So I do understand and appreciate the arguments made thus far against this practice. I still enjoy this practice it because it makes it so much easier to just #include "defs.h" and then not have to worry about declaration lists any further. I guess it's caused me to become a little bit lazy. rolleyes.gif

 

You should be very careful with anything that makes you lazy, especially when it directly affects the code structure.  There's a difference between being lazy and being efficient, although superficially they can seem similar.  But, efficiency almost always leads you in the right direction, while laziness almost always leads you in the wrong one.

 

A good dependency structure should look like a tree.  Yours kind of looks like a circle... graphics depends on something that depends on audio, which depends on something that depends on graphics, ...

Share this post


Link to post
Share on other sites

It's a rather bad practice. At my last company we ended up with, for similar reasoning, a file known as "gameutil.h". It literally contained every function you could think of relating to gameplay mechanics. Many of those were inline because they were just short math functions for interpolations, springs, etc.. Instead of being sane, and having "interpolate.h" and "spring.h" and whatever else would make sense, it was all inside "gameutil.h". Changing one file yields 10-30min down the drain waiting for a compile, and that is NOT what you consider fast iteration times. In the end everyone hated it, but it was too big of a pain in the ass to refactor out.

 

Now I work at a place where that type of thing is forbidden by pre-checkin scripts looking for "one top-level class per header". And checks for un-used includes. When you find yourself looking to save a few keystrokes you're probably using the wrong tools. Find, or build, something to automate what is giving you a hard time. There are a few IDEs with good refactoring ability that will save you "keeping forward declarations in sync" and a few tools out there that graph your include hierchy and spit out lists of extraneous includes. I've used include manager in the past, and as long as your code-base isn't a complete mess to start with it helps.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!