0
\$\begingroup\$

Hello I am trying to teach myself about the how's and why's of OOP in C++. So I am trying to seperate most of my code into specific classes and methods that do specific things.

I've also been reading/watching lots about best practice for OOP, and I can see how it gives a great advantage for making changes to things later, or re-using classes/components in other projects or other game objects.

From what I understand it is NOT good practice to put #include's to all the needed header files , inside my header files. Rather I should 'forward-declare' them in .h and then fully define them in the .cpp files. This will avoid cycles of header pastes, which gives compiler errors.

However there are MANY threads on SFML help forums directly contradicting this advice. https://en.sfml-dev.org/forums/index.php?topic=21705.0 , https://en.sfml-dev.org/forums/index.php?topic=2043.0 (and more). Thing is, I ran into compiler errors doing it their way. And had to use #pragma once everywhere (even in very small project).

I found a way to properly forward declare (https://stackoverflow.com/questions/13848451/how-to-forward-declare-a-c-template-class/13848492), and it works in the most part. Here's where it gets wierd for me....

My Guy class, it works . I have successfull forward declared a RenderWindow, Texture, Sprite and Vector2 all in sf namespace:

Guy.h:

namespace sf
{
class RenderWindow;
class Texture;
class Sprite;
template <typename> class Vector2;
}

class Guy
{
public:
Guy();
void Start();
void Update();
void Draw(sf::RenderWindow& Window);
const sf::Vector2<float> GetPosition();
void SetPosition(float x, float y);

private:
sf::Texture Texture;
sf::Sprite Sprite;
};

Guy.cpp:

#include "SFML/Graphics.hpp"
#include "SFML/System.hpp"
#include "Guy.h"
#include "Constants.h"


Guy::Guy()
{
Texture.loadFromFile("Textures/guy.png");
Sprite.setTexture(Texture, true);
Sprite.setOrigin(Sprite.getTextureRect().width * 0.5f, Sprite.getTextureRect().height * 0.5f);
SetPosition(15 * Constants::PPU, 18 * Constants::PPU);
}

void Guy::Start()
{
}

void Guy::Update()
{
}

void Guy::Draw(sf::RenderWindow& Window)
{
Window.draw(Sprite);
}

const sf::Vector2<float> Guy::GetPosition()
{
return Sprite.getPosition();

}

void Guy::SetPosition(float x, float y)
{
Sprite.setPosition(x, y);
}

That all worked for me, but then I tried to move the #includes on my 'LevelManager' class (this time I had to forward declare stuff from sf and std namespace, maybe this is why?)...

LevelManager.h:

namespace sf
{
class RenderWindow;
class View;
class Texture;
}

namespace std
{
class string;
template <typename, typename> class vector;
}

class Guy;
class Tile;

class LevelManager
{
public:
void Start();
void Update();
void Draw(sf::RenderWindow& Window);
sf::View& GetGameView();

private:
sf::View GameView;
Guy Guy;

std::string Level1;
sf::Texture TileTexture_Sky;
sf::Texture TileTexture_Ground;
std::vector<Tile> LevelTiles;
};

LevelManager.cpp:

#include <string>
#include "SFML/Graphics.hpp"
#include "Guy.h"
#include "Tile.h"
#include "Constants.h"
#include <iostream>
#include "ViewTools.h"
#include "LevelManager.h"



void LevelManager::Start()
{
Level1 = "";
GameView.setSize(Constants::VIEW_WIDTH, Constants::VIEW_HEIGHT);
GameView.setCenter(GameView.getSize().x / 2, GameView.getSize().y / 2);
GameView = ViewTools::GetLetterBoxView(GameView, Constants::WINDOW_WIDTH, Constants::WINDOW_HEIGHT);



Guy.Start(); 
GameView.setCenter(Guy.GetPosition());

TileTexture_Sky.loadFromFile("Textures/tile_sky.png");
TileTexture_Ground.loadFromFile("Textures/tile_ground.png");

// .......Rest of class...
// ......
// ......

And this gives me hundred+ errors. It got very hard for me to debug, because it seems on Guy.cpp if I re-order the #includes , this can break them. They needed to have SFML/Graphics.hpp first I think (?I'm confused?). I tried many orders in LevelManager but couldnt get rid of errors. I also tried many variations of the wordings for forward declaration without success.

If I #include SFML/Graphics.hpp (and Guy.h & Tile.h) in LevelManager.h , then the problem is gone... but one day I will have to include it in two headers unless I realise why it is like this.

The SFML forums where not super-helpful. They seem to tell you just do it in .h and use #pragma once.

My Question: Should I find a way to #include "SFML/Graphics.hpp" and the other two headers in the LevelManager.cpp, or move them back to the LevelManager.h? I do also have other queries about why sometimes the forward declarations work fine, and other times they don't (but I cannot get the question written concisely, i will have to form a new question for this and post all the code i think).

At any rate, thank you for all and any help

\$\endgroup\$
  • 1
    \$\begingroup\$ I recommend that you do not use forward declarations for any library type: If the library decides to change from class to struct (or typedef), you need to adapt all forward declarations. Additionally, the C++ standard says that you should not use forward declarations for std types, e.g., std::string or std::vector. \$\endgroup\$ – pschill Feb 19 at 12:28
  • \$\begingroup\$ thank you for the extra info. I am going to not use forward declaration until i find out myself a very good reason to use them. I will use pragma once and the include guards as discussed in the answers here (nb. I haven't yet done the include guards, just the #pragma once \$\endgroup\$ – Krangogram Feb 19 at 21:39
1
\$\begingroup\$

My suggestion is to

  1. Use include guards on your .h files
  2. Move necessary #include directives to these .h files

A .h file is an interface to describe some other compiled binary file. If I want to include Guy.h in LevelManager.cpp, I shouldn't have to comb through Guy.h to see which other #includes I need to add to LevelManager.cpp to get Guy.h to not need forward declarations.

Guy.h should be set up so that any other includes it needs are, well, included in Guy.h. That means at a minimum you should move #include "SFML/System.hpp" and #include "SFML/Graphics.hpp" to Guy.h. #include "Constants.h" isn't necessary in Guy.h since it's unused.

But what about circular dependencies?

To avoid circular dependencies, you simply tell the compiler to only include that file once. If you use a compiler that enables #pragma once, you can use that, but more universal is the use of include guards.

If you look at SFML/System.hpp you can see an example of these at the top (and very bottom) of the file:

#ifndef SFML_SYSTEM_HPP
#define SFML_SYSTEM_HPP
...
#endif // SFML_SYSTEM_HPP

My suggestion is to move the minimum number of #include statements to Guy.h and LevelManager.h so that you don't need to use forward declarations, and to sandwich the entire files in include guards. Naming standard is #ifndef ALL_CAPS_ENDING_IN_AN_H

Guy.h becomes:

#ifndef GUY_H
#define GUY_H
...
#endif // GUY_H

LevelManager.h becomes:

#ifndef LEVEL_MANAGER_H
#define LEVEL_MANAGER_H
...
#endif // LEVEL_MANAGER_H
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ thank you i will try to apply this to the project \$\endgroup\$ – Krangogram Feb 19 at 1:17
1
\$\begingroup\$

I'll not cover what has already been said it the other answer as it's good advice (#pragma once should be the first thing seen in each of your own .h files; in headers, include what the header actually uses, forward declare what it only tell it uses (e.g. only used as pointers) and include in the cpp [note that there is some debate about it, and I think Google's coding standard says to not forward declare and always include...]).


Forward declaring in SFML C++ . How to know whether to #include in .h or .cpp?

A solution is to not include them anywhere. Or everywhere.

You can look into precompiled headers. This is a feature that will let you group all of the most used headers in your project, will pre-compile them when you compile your project the first time and use them "automatically" anywhere you need them, without you needing to type the #include <sfml/bla.h>.

The main advantages of the precompiled headers include:

  1. Include the header once.
  2. Access the included content anywhere in your project.
  3. In the long run, faster compilation for your project.

The drawbacks in using precompiled headers:

  1. You need to set them up in your project configuration
  2. You need to include it in each of your .cpp file in your project, and this include has to be the first thing to include. (At least in Visual Studio.)

A typical use of the precompiled headers is to include in it all the library headers (because those change, like, well, never) including std, windows, and in your case, sfml. So, basically, if you're to include something with the angle brackets (< and >), you should include it in the precompiled headers (and only there, no need to repeat yourself). And you include your own files the typical way.

For instance, your LevelManager.h file could look like this:

#pragma once

#include "Guy.h"
#include "Tile.h"

class LevelManager
{
public:
  void Start();
  void Update();
  void Draw(sf::RenderWindow& Window);
  sf::View& GetGameView();

private:
  sf::View GameView;
  Guy Guy;

  std::string Level1;
  sf::Texture TileTexture_Sky;
  sf::Texture TileTexture_Ground;
  std::vector<Tile> LevelTiles;
};

While the .cpp would look like that:

#include "precomp.h" // <- Configured in your project, contains all the headers you want precompiled.  

#include "LevelManager.h"
#include "Constants.h"
#include "ViewTools.h"

void LevelManager::Start()
{
  Level1 = "";
  GameView.setSize(Constants::VIEW_WIDTH, Constants::VIEW_HEIGHT);
  GameView.setCenter(GameView.getSize().x / 2, GameView.getSize().y / 2);
  GameView = ViewTools::GetLetterBoxView(GameView, Constants::WINDOW_WIDTH, Constants::WINDOW_HEIGHT);

  Guy.Start(); 
  GameView.setCenter(Guy.GetPosition());

  TileTexture_Sky.loadFromFile("Textures/tile_sky.png");
  TileTexture_Ground.loadFromFile("Textures/tile_ground.png");

  // .......Rest of class...
  // ......
  // ......

* I changed a bit the order here, and it's a preference, lots of debate around the Internet.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thank you. The "precomp.h" seems very interesting. Would "SFML/Graphics.hpp" go into this precomp.h file too? \$\endgroup\$ – Krangogram Feb 19 at 2:35
  • \$\begingroup\$ @Krangogram Yes. The way you seem to be using the library, it seems that you have put it directly in your project. Typically, this will be set up as an exterally compiled library, with the header files #included. You would then include all of the SFML headers into that precompiled header. \$\endgroup\$ – Vaillancourt Feb 19 at 2:38
  • \$\begingroup\$ Ah ok I am confused about that also. When i began the project, I went through the tutorial about setting Linker settings and adding Compiler (in the Visual Studio project settings menu). I don't think I manually dropped the sfml files into my project. I dont remember exactly though as I created a SFML template and use thast now to begin new projects \$\endgroup\$ – Krangogram Feb 19 at 2:40
  • 1
    \$\begingroup\$ Ok, then if you use SFML as a library, you might want to use #include <SFML/Grahics.hpp> instead of #include "SFML/Graphics.hpp". I think there is an actual technical difference whether you use one over the other, and it also conveys "this is from an external library" or "this is in this project". \$\endgroup\$ – Vaillancourt Feb 19 at 2:44
  • \$\begingroup\$ wow thank you , i wasn't even aware i could do that. I wish i could mark both answers as correct. I'm not sure which is best but i appreciate the help from both of you \$\endgroup\$ – Krangogram Feb 19 at 2:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.