Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: Can someone review parts of my Breakout clone code?  (Read 3776 times)

0 Members and 1 Guest are viewing this topic.

TCVM

  • Newbie
  • *
  • Posts: 30
    • View Profile
Can someone review parts of my Breakout clone code?
« on: April 21, 2016, 01:45:08 am »
I have just finished my own little polished clone of Araknoid/Breakout, and I was wondering if anyone would mind reviewing parts of my code.

I want to know if my usage of "global" variables is proper. Right now I have a class in globals.hpp that has many public static members of things that I use around my program; resource managers, input managers, etc. Any class that includes "globals.hpp" has access to all of these files. I did this because I really don't want to be passing objects through classes that don't need them just to give them to something else

The other part of my code I want to hear about is my observer pattern. Currently I have an observer pattern with observers and the dispatcher. Many objects inherit from observer and then subscribe to events that can occur. I am wondering if I am using this to liberally, or if it's fine.

Thanks in advance!

Code:
https://github.com/TheCandianVendingMachine/Breakout_TCVM/tree/master/src

Parts in question:
 Globals - game/globals.hpp
 Observer pattern - managers/events
 Use of an observer - states/gameState.hpp

eXpl0it3r

  • SFML Team
  • Hero Member
  • *****
  • Posts: 10815
    • View Profile
    • development blog
    • Email
Re: Can someone review parts of my Breakout clone code?
« Reply #1 on: April 21, 2016, 08:58:07 am »
I want to know if my usage of "global" variables is proper.
The use of globals is by itself improper. ;)

Right now I have a class in globals.hpp that has many public static members of things that I use around my program; resource managers, input managers, etc. Any class that includes "globals.hpp" has access to all of these files. I did this because I really don't want to be passing objects through classes that don't need them just to give them to something else
Then you should design your classes differently. Globals break the scope and encapsulation. It makes it hard to reason about the state of an application, since anything in the code could potentially have side effects on the globals. And finally they can cause a lot of problems if not initialized/destroyed properly, because the destruction of objects in the global scope is undefined.

If a lot of your objects need access to these global classes, you might think about, how you could reduce the dependencies. For example could the class holding the other classes construct these classes? Thus these "child" classes won't have to know about for example the texture manager? (Factory pattern comes to mind)
By reducing dependencies you only have to pass these "globals" to a small amount of other classes, but not only that, you will also get a cleaner code base. ;)
Official FAQ: https://www.sfml-dev.org/faq.php
Official Discord Server: https://discord.gg/nr4X7Fh
——————————————————————
Dev Blog: https://duerrenberger.dev/blog/

TCVM

  • Newbie
  • *
  • Posts: 30
    • View Profile
Re: Can someone review parts of my Breakout clone code?
« Reply #2 on: April 21, 2016, 07:23:00 pm »
Alright, I get it now. But what should I do about objects that don't belong anywhere? Let's say the inputManager. It needs to be referenced in completely unrelated parts of the code such as the menu state, game loop, and player. None of these classes are related, but I still need to pass the inputManager to them. Or a logger, it needs to be in virtually every class, but passing around a single pointer seems unneeded.

eXpl0it3r

  • SFML Team
  • Hero Member
  • *****
  • Posts: 10815
    • View Profile
    • development blog
    • Email
Re: Can someone review parts of my Breakout clone code?
« Reply #3 on: April 21, 2016, 07:29:09 pm »
A log class can (doesn't have to be) one of the exceptions regarding globals.

As for the inputManager, I'm not quite sure what it's use case is. A lot of the dependencies could be broken up by using a message queue. Maybe you could expand your observer pattern thingy?
Official FAQ: https://www.sfml-dev.org/faq.php
Official Discord Server: https://discord.gg/nr4X7Fh
——————————————————————
Dev Blog: https://duerrenberger.dev/blog/

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32504
    • View Profile
    • SFML's website
    • Email
Re: Can someone review parts of my Breakout clone code?
« Reply #4 on: April 21, 2016, 08:04:24 pm »
Global access doesn't mean global variable. You can still (and should!) create those in a well-defined order at a well-defined place. Which doesn't prevent you from giving global access to them with a free function or whatever.
Laurent Gomila - SFML developer

TCVM

  • Newbie
  • *
  • Posts: 30
    • View Profile
Re: Can someone review parts of my Breakout clone code?
« Reply #5 on: April 21, 2016, 09:12:19 pm »
Alright, I think I have figured out what I need to do. So if I need an object to be used in multiple places (logger, input manager, etc), I will use a global pointer to the object which would be defined in a non-global namespace. And the objects that don't need to be accessed by everything (if needed), will be members of another class, probably living in a factory class. Correct me if I am wrong

Thanks for the advice!

Tukimitzu

  • Full Member
  • ***
  • Posts: 117
  • Anti-Hero Member
    • View Profile
Re: Can someone review parts of my Breakout clone code?
« Reply #6 on: April 21, 2016, 09:57:41 pm »
You got it.
These are two ways I use globals in my current project:
1) I have a EntityManager object that all systems need access to, so all the systems keep a pointer to that object.
2.1) I have a static class called Resources where I keep images, sounds, musics and fonts. There's no such thing as "static class" in C++, but if you declare all the methods and members as static, BAM!, you got a static class.
2.2) For the Resources class I could also instead of static class use a Singleton Design Pattern, but this is too fancy for a one-man project (IMO), which seems to be your case.

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32504
    • View Profile
    • SFML's website
    • Email
Re: Can someone review parts of my Breakout clone code?
« Reply #7 on: April 21, 2016, 10:23:52 pm »
Quote
There's no such thing as "static class" in C++, but if you declare all the methods and members as static, BAM!, you got a static class.
If you talk about singleton, then this thing would probably be the monostate pattern.
Laurent Gomila - SFML developer

TCVM

  • Newbie
  • *
  • Posts: 30
    • View Profile
Re: Can someone review parts of my Breakout clone code?
« Reply #8 on: April 21, 2016, 11:20:06 pm »
Alright, thanks for the advice everyone

 

anything