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

Started by
9 comments, last by ApochPiQ 11 years, 3 months ago

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]

Hero of Allacrost - A free, open-source 2D RPG in development.
Latest release June, 2015 - GameDev annoucement

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.

if you think programming is like sex, you probably haven't done much of either.-------------- - capn_midnight

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.

Hero of Allacrost - A free, open-source 2D RPG in development.
Latest release June, 2015 - GameDev annoucement

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.

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

Hero of Allacrost - A free, open-source 2D RPG in development.
Latest release June, 2015 - GameDev annoucement

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.

I can't comment on your solution itself, but I'll address some of your reasons for the solution. If there are alternative solutions that are just as easy, that don't have the side-effect of huge compile times rippling through the codebase, you could decide for yourself if they are worth switching to.

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.

An understandable concern - it could be a good programmer-discipline standardized for your project to Ctrl+F the entire project for occurrences of that class name. (A find-and-remove of "class AudioEngine;"). If a programmer forgets to do this, then those declarations don't do any harm, since the code doesn't reference them, and they'll be discovered and removed by Ctrl+F-ing the entire project when they are later discovered.

Or when a class gets renamed, adding in its new declaration and forgetting to remove the old one.

I use QtCreator as my IDE. To rename a class, I right-click on the class name and do "Refactor -> Rename symbol under cursor", and it renames every usage, declaration, and definition of that class throughout the entire project effecting only the files and headers that use it. You can do the same with function or member-function names, and variable instances and member-variable names. It's very useful - are you sure your IDE doesn't have a similar feature?

Alternatively, the old "Find-and-replace" project-wide works just as well and has the same effect (with the added bonus that it even catches usage within comments).

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).

Which is a fantastic habit to have. smile.png

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.


An alternative, which partially addresses ChaosEngine's complaint, and partially-addresses the compile-time complaint is to have system-specific "Declaration" headers. "AudioDeclarations.h", "VideoDeclarations.h", "CoreDeclaration.h", etc... A change to the audio declaration only effects the audio-subsystem and the non-audio files that include audio-headers. It'd still be easy and maintainable, but also properly keeping the different sections of the project unaware of even each other's pre-declarations, except when explicitly included. Such declarations should also have any system typedefs, unless they are class-specific typedefs which are within the class' namespace.


The standard C++ library has a header file <iosfwd> that forward-declares alot of the iostream templates and classes.

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

I restore Nintendo 64 video-game OST’s into HD! https://www.youtube.com/channel/UCCtX_wedtZ5BoyQBXEhnVZw/playlists?view=1&sort=lad&flow=grid

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, ...

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.

This topic is closed to new replies.

Advertisement