the public syndrome

Started by
19 comments, last by dworm 10 years, 3 months ago

So one thing that gave me trouble was that I always confused "You should only make what needs to be public public" with "You shouldn't have very many public things".

You may actually *Need* quite a few public properties/methods etc.

Imagine writing a library... the library is meant to perform a particular task (or set of tasks) configuring and telling the lib what to do may require a large amount of public properties/methods... but anything that is not required to configure or utilize the lib should not be public. Take C# windows.forms for example, the are hundreds of public properties/objects... but every single one of them performs some kind of data transfer from the assembly using the lib to the assembly implementing the lib... you have to know that there are even more objects/methods/members that the lib doesn't expose... if it had exposed all the things the end user doesn't need to know about it would be that much more difficult to utilize.

You also have an internal scope to work with as well. Internal scope is pretty much equivalent to public when working within the same assembly.

Just remember, scope and limiting scope is just a tool meant to help you organize your code, you won't see much (if any) performance difference between an object properly scoped and one that is entirely public, but you will find it easier to figure out how it's supposed to work/ how it is working.

The reason the rule is in place is because you want to make code as independent as possible.. Objects should only be concerned with their own particular responsibilities.

what if i have to read the mouse click while checking if i have a menu/tooltip/mouseover open (inside my menu class) and then checking if my mouse position correspond at a game location or a interface (inside my mouse class) and then checking if the mouse position in game correspond to an interacting object(so i have to check map class and object item) ?

The menu/tooltip shouldn't have to be responsible of keeping track of the mouse... it is only concerned with Displaying and allowing the user to interact with a menu... it shouldn't even care what happens when a particular option is selected. as long as it reports that an option has been selected it's done it's job.. something else can take care of what to do about it. A good way to acomplish this is to use events, Object O spawns a menu, it sends it a list of options to display and a list of delegates that corrospond to each option, the menu displays the options and if one is selected calls the appropriate delegate.

from my beginner point of view it seems equally bad design making a class "tree" and then implement in that class "enemySeesTree" "enemyAttackTree" "pahtingAvoidTree" "playerClickTree" "playerShotTree" and other 200 like this...

Remember, every distinct action/logic HAS to be defined Somewhere... you can only generalize to a point and somewhere along the line actual implementation has to occur. In this case I'd probably have events: onSighted(object sightedBy), onClick(), onCollision(object source) pathing would likely not be an event, but instead a method that return a pathing weight... getPathWeight(object pathingObject)

Advertisement

by the way "components" in general are just structs of data so there are no getters and setters -

only one question: why structs? from my understanding they are supposed to be used for data you dont change much and that is small

structs and classes, in C++ at least, are exactly the same thing, with only one trivial difference: structs default members to public, and classes to private. (There's also some differences in how they handle inherited members, but again, it only has to do with publicity, not functionality.)

What you use is entirely a matter of personal preference. Many coding standards encourage using structs for simpler data types, but that's just an arbitrary convention.

Wielder of the Sacred Wands
[Work - ArenaNet] [Epoch Language] [Scribblings]

For me personally, it should read basically like spoken language: subject verb objects.


Classes should be nouns (the subject), functions should be verb (an actions) and parameters should be other objects used by the action.

Although accessors and mutators (sometimes called getters and setters) follow the rule they are extremely inefficient. When you want to tell a person to go somewhere you don't give them instructions about moving each foot or how to balance their weight as they walk, instead you give the very broad instruction to go somewhere and let them grapple with the details.

If you need to give all those details, and your code looks like something.GetValue().GetValue().GetValue().DoSomething(), that is a problem. If you are trying to pass around too many details, like something.DoSomething(foo.GetValue().GetValue().GetValue()), that is a similar but different problem. In both cases you are trying to micromanage the objects rather than giving broad instructions.

If you have a bundle of data create a plain old data structure (POD) and send it over as an object. If you have a thing that needs to be manipulated, make it a proper class with verbs to manipulate it at a high level. Try to pass whole objects; send the entire actor and not just his feet, or send a pointer to the UI pointer rather than a few tidbits about the UI's state.

The only thing that needs to know about the mouse is the UI code. All interactions with the user, the mouse, keyboard, gamepad, wheel, or any other control device, should be constrained to the UI code. The rest of the game should not care where the mouse is at, or what windows or visible, or if the mouse axis is reversed as a player preference; the UI should translate all of that information to events. There is almost never a good reason for a game object to need details about the user interface; they can send events back and forth but that should generally be the extent of it.

Using globals is normal as a beginer, the more code you will write, the more tricks you will learn to avoid them, until you eventually only use the strict minimum. A few globals is not bad, sometime they are even required, but the less you use, the better.

For me personally, it should read basically like spoken language: subject verb objects.


Classes should be nouns (the subject), functions should be verb (an actions) and parameters should be other objects used by the action.

Although accessors and mutators (sometimes called getters and setters) follow the rule they are extremely inefficient. When you want to tell a person to go somewhere you don't give them instructions about moving each foot or how to balance their weight as they walk, instead you give the very broad instruction to go somewhere and let them grapple with the details.

If you need to give all those details, and your code looks like something.GetValue().GetValue().GetValue().DoSomething(), that is a problem. If you are trying to pass around too many details, like something.DoSomething(foo.GetValue().GetValue().GetValue()), that is a similar but different problem. In both cases you are trying to micromanage the objects rather than giving broad instructions.

If you have a bundle of data create a plain old data structure (POD) and send it over as an object. If you have a thing that needs to be manipulated, make it a proper class with verbs to manipulate it at a high level. Try to pass whole objects; send the entire actor and not just his feet, or send a pointer to the UI pointer rather than a few tidbits about the UI's state.

The only thing that needs to know about the mouse is the UI code. All interactions with the user, the mouse, keyboard, gamepad, wheel, or any other control device, should be constrained to the UI code. The rest of the game should not care where the mouse is at, or what windows or visible, or if the mouse axis is reversed as a player preference; the UI should translate all of that information to events. There is almost never a good reason for a game object to need details about the user interface; they can send events back and forth but that should generally be the extent of it.

so this is very bad eh?

for (int j = 0; j < itemTypes[gameObjects.Type].NumberOfAction ; j++)
{
gameWindows[menusNumber].addWindowText(j + 1, Game1.itemTypes[gameObjects.Type].ItemName);
}

so this is very bad eh?

for (int j = 0; j < itemTypes[gameObjects.Type].NumberOfAction ; j++)
{
gameWindows[menusNumber].addWindowText(j + 1, Game1.itemTypes[gameObjects.Type].ItemName);
}

Yep.

Consider this (almost identical) version.


  for (auto const& game_object: gameObjects)
  {
    for (auto const& item_type: itemTypes[game_object.type()])
    {
      gameWindows[menusNumber].appendWindowText(item_type.name());
    }
  }

The difference? (1) instead of externally indexing text items, they're just appended and the gameWindows can keep track of the index (append(text) instead of add(index, text)), and (2) const getters for the object type and item type name properties. Oh, and slightly different flow control structure, because I want to be able to look at a line of code and understand what it does, not spent 10 minutes parsing it so I can struggle to gleam its meaning. To me, this code says more clearly "for each game object, get all its types and append their names to the menu."

I'd actually go farther and try to eliminate anything that talks about "types," since that tends to be a code smell telling me to move from procedural/declarative coding style into OO coding style. Then again, I come from the era before OO and I'm familiar with its benefits over procedural/declarative.

Stephen M. Webb
Professional Free Software Developer

An object should be correct by construction, and its public API should only produce valid states as well.

If you have a Point class with two public fields X and Y, that’s fine as can be—there are no invalid Points, so there are no invariants to preserve, and the fields can be exposed directly. Whereas if you have a Heroine object with Health and Shield fields, then it does not make sense to expose these directly, because code could place the heroine in an invalid state by killing her even though she has a shield. That’s why people recommend methods like “heroine.takeDamage(monster.strength)”.

The other thing to consider is that if you use immutable objects—that is, treat them as values representing the state of an object, not as the state itself—then as long as objects are correct by construction, they cannot ever be in an invalid state because they will never change after construction. That can make it significantly easier to reason about what your code is doing and ensure its correctness.

An object should be correct by construction, and its public API should only produce valid states as well.

If you have a Point class with two public fields X and Y, that’s fine as can be—there are no invalid Points, so there are no invariants to preserve, and the fields can be exposed directly. Whereas if you have a Heroine object with Health and Shield fields, then it does not make sense to expose these directly, because code could place the heroine in an invalid state by killing her even though she has a shield. That’s why people recommend methods like “heroine.takeDamage(monster.strength)”.

ok my problem is combining what you say that makes sense with my exemples aboves

lets add a shield made with iron

and make a situation where the monster ai attack only if heroine has a wooden shield

so it end up with monsterAi calling heroine.checkShield(wood) who will make the actual check

while im trying to reason this way and i see a few advantages , my naive logic push me to a more intuitive if shield inside monsterAi

It makes sense for the monster to check the player’s shield when it attacks, because that’s what the monster is doing in the simulation. It makes sense for the player object to have a shield object, because that’s exactly the situation you intend to represent. You could have a public member function on the player for getting a read-only view of their equipment—then monsters could use those methods to answer questions like “Does the player have a shield, and is it wooden?” but wouldn’t be able to alter that information in unstructured ways.

An interaction like this can be modelled in many ways. It’s up to you to start with something plausible, and alter it when you see the need to. And you can’t really predict the choices you will make in the future, so it doesn’t make sense to write much more than just what gets the job done well in the present and is easy enough to change later.

OOP is a class of architectural philosophies, not a consistent theory with specific or consistent rules, so there is no “right answer” here. You just have to develop a sense of design aesthetics over time. One good way to do that is to have other developers review your code and point out areas for improvement. But I would say correctness by construction is a very strong guideline.

This topic is closed to new replies.

Advertisement