Using Role as Partial Classes
For Veure, my text MMORPG, I found myself worrying about the Character class turning into a god object. At over 2,300 lines, 105 methods, and growing, it was getting hard to keep track of everything. I want my code to be clean and easy to follow. As a result, when new behavior arose, I started thinking of other places to put behavior, and wound up with abominations like this:
if ( $missions->has_misssion_for( $character, $npc ) ) {
...
}
If you're not familiar with the terminology, NPC stands for "non player character" and, as you might guess, they're implemented via the same class as player characters. So what does the above code do? Does it mean that your character has a mission for an NPC? Does it mean that an NPC has a mission for your character? Does it mean that your missions have a mission for ... wait, what does that even mean? The last one, though, is the natural reading of the above.
All because I was trying to limit the size of the Character class.
But wait, this is an MMORPG. Your character drives everything. Your character moves. Your character goes on missions. Your character has to interact with their inventory. Your character has to do all sorts of things and artificially shoving those things into different classes just to avoid the "god object" makes for unclear code like the above. However, shoving all of that behavior into the Character class means it's getting huge and unreadable.
Until now.
I don't want to get too experimental with the programming of Veure. I want solid, proven concepts. However, I had hit a wall with the Character class when I was implementing missions. There was so much stuff going on that I needed a better organization and while trying to constrain missions to the smallest scope possible (you don't want special cases for mission code littering your entire code base), I found myself pushing a number of mission-related methods into the burgeoning Character class because, damn it, that's where they belonged:
if ( $npc->has_missions_for($character) ) {
...
}
There is zero ambiguity in the above code, god objects be damned (er ...).
If you're familiar with the concept of a partial class, it's a way of splitting a class's implementation across several files. It's often used with auto-generated code so that the generator can write code to a partial class and it gets compiled into your class later. That's what I started implementing today, but using roles for it.
Roles, as you know, can let you safely share code across classes which are unrelated by inheritance. I didn't want to share my Character code with different classes, I wanted to decompose it into logically related components. Now, near the top of my Character class, you'll see the following:
with qw(
Veure::PartialClass::Character::Inventory
Veure::PartialClass::Character::Mission
Veure::PartialClass::Character::Movement
);
Each of those is implemented as a role, of course, but I call them PartialClass instead of Role because that's what they are. That's shed a number of methods from the Character class and about 600 lines of code. More will be handled later.
This has a number of interesting benefits. Aside from logically grouping related parts of classes (albeit in different files), I can now take advantage of the requires functionality of roles:
package Veure::PartialClass::Character::Inventory;
use Moose::Role;
requires qw(
_get_schema
character_items
inc_bank
);
Now, if I accidentally delete a required method in a disturbingly huge class, the class might fail to compile instead of me hoping my test coverage would catch it (yes, it would, but you get the point).
I should have the first pass of missions merged into the master branch of Veure soon. They're still a bit more complicated than I like, but the open-ended nature of them means that if I wanted to, I could approach the complexity of text adventures with them. Instead of "go kill 5 things", how about solve a mystery? My mission system would support that (though it would be a hell of a lot more work to write).
Freelance Perl/Testing/Agile consultant and trainer. See http://www.allaroundtheworld.fr/ for our services. If you have a problem with Perl, we will solve it for you.
And don't forget to buy my book! http://www.amazon.com/Beginning-Perl-Curtis-Poe/dp/1118013840/
To me, those roles sounds like a degenerate case of the normal "roles for code reuse" approach. Degenerate, because they are only composed into one class.
But who knows, maybe in future there's something other than a character that does movement? (a vehicle maybe?)
Actually, for everything you have written i would recommend my own blog that i have written that exactly faces the problem you have.
https://blogs.perl.org/users/sid_burn/2014/07/inheritance-is-bad-code-reuse-part-iii.html
But it probably makes the sense to read all three parts.
The overall problem that you described here and that i see is that when you started you didn't think in terms of "Behaviours". Then with roles you started to put everything into Roles and more like Behaviours. But in my opinion, you should drop Roles completly. In Part II of my blog i describe something what you have here, and in Part III i replace Roles with Composition. In fact, that is a lot better as using Roles.
On top of that, you still don't really have Behaviours. "Movement" for example should be a behaviour that is re-usable. because in Movement there should absolutly nothing that is depending on your Character.
A Character should have a Movement object. And this object alone handle everything that is related with movement. Everything related to movement should be in that class. That means, it has a current Position and you have your different methods to change the movement.
Actually there shouldn't really anything in your Movement class that needs a character. Because that violates the Single responsibility principle.
If you design it that way, you end up with a Movement class that is reusable. If you also want an NPC, Vehicle or absolutly any other gameobject to move in your would, you just give it is own instance of a Movement object.
Also an inventory is not really tied to a character. Also friendly NPCs or enemies could have an inventory. For example some friendly NPC that sells you stuff should also just use an inventory. Even if the inventory of someone that you can buy stuff have another look, it doesn't change that you can and should ise an inventory for it. How it looks is part of the "View". The inventory itself is the "Model". Even a chest in your would should just use an inventory. Because thats the whole point of an inventory. It can contain multiple items.
You end up with a lot better of code-reuse, but also the whole thing what you programs will be easier. Because removing an item from the character inventory and transfering it to a chest should be a no brainer because it has the exact same interface and accepts the exact same classes/types.
I can highly can recommend what i have written in the blog i linked above. Because everything what i explained here is explained there in a lot more detail.
Thanks for your comments, Sid. I saw some interesting things in the second part of your series of posts and I made some comments there about them.
Next, you wrote "Movement" for example should be a behaviour that is re-usable.
This is not true. I have no other need for movement at this time, so solving a problem I don't have is a very bad practice. I'm building an MMORPG and if I try to solve every problem I don't have, I'll never finish. If, and only if, I need to re-use movement behavior, will I solve that problem.
You also wrote: A Character should have a Movement object. And this object alone handle everything that is related with movement. Everything related to movement should be in that class. That means, it has a current Position and you have your different methods to change the movement.
Again, if I find I need to reuse this behavior, maybe I'll go that road, but it would be a role, not a class. Making classes for no other purpose than saying "I have something to delegate to" doesn't make any sense. Instead, we're talking about shared behavior and that's what roles were created for: decoupling responsibility and behavior. When designing classes, you're generally looking for your top-level business entities: customers, products, orders, warehouses, and so on. You don't want to make the naïve mistake of trying to mirror them in your software (you only create functionality you actually need and that fits your problem domain), but by conceptualizing your top-level concerns as classes, you make your system easier to comprehend.
You don't make "movement" a class. "Movement" is a behavior with some state attached to it. My top-level business entities in Veure are stars, space stations, items, characters, and so on. Not movement. Not "inventory management." Not "running missions" (though the missions themselves are). That being said, your inventory comments make sense and it's certainly something I may revisit later, but for now, I don't have that problem. (Though the rename the PartialClass to a Role would be trivial because the behavior there is quite shareable).
Also, while your comments about the single responsibility principle are good for the theoretical case, they often break down in the real world, as explained on the C2 wiki.
This is not true. I have no other need for movement at this time, so solving a problem I don't have is a very bad practice.
That you can re-use it, is more a side-effect. Movement is a single responsibility, so it should be its own class. The benefit of it is that you create small classes that are easy to understand. And because it is a class you also can better test them. Actually, you can't really test a Role. Sure, you can ignore that principle, but those principles doesn't exists to be a burden on you, those are principles that especially helps you to write better code for large projects. And i consider your MMO exactly like that. If you don't follow these principles, what you sure can do, it will later backfire at you because the bigger your project gets, you will see that it will get harder and harder to introduce new features.
I would say. Especially because you program an MMO and you program should be finished in the future you should exactly do this. Actually there isn't even a lot of work here. I mean creating a role or are class is nearly the same amount of work! For a class you use "use Moose" for a role "Moose::Role". And you still have to include your role in your character class, as you would do with an Attribute.
It by the way doesn't mean that you have to create every class from the beginning, but if you refactor, refactor it in classes instead of roles. If you refactor like this you will start to see the benefits. Just a quick example. In your current design Movement is just a role included in your charcter. Lets assume you want a pet that follows your character. What you probably will do in your code is pass a reference from your character in your pet. So the pet is able to read the positions of your character and follow you.
Let's assume you want a new functionality that you can give your pet orders like: Go to this point!. Stay Here! and so on. In your current design you can't really do that because your logic was written to follow a whole character. The result is, a massive refactoring of your code and rewriting a lot of logic. If you do it the composition way you would probably just end with a "Position" class or better a Vector2 or Vector3 that represents a position. Your pet would never follow a "character". A pet would follow a specific point. If you want that your character follows the location of your character you would write.
$pet->movement->position($character->movement->position)
If you want it to follow you update the position of the pet. If you want that the Pet stays at a position. Then you don't update the psoition of the pet. If you want that your position follows something else or runs to a specific position. You just create suche a Position.
$pet->movement->position(Vector2->new($mousePosition->x, $mousePosition->y))
$pet->movement->position($thatTree->position)
Now, you say you don't want to solve problems that you don't have. My answer is more. You will automatically get this benefits because you just create classes instead of roles. I mean i often compare it to database design. You often do normalization. You normalize your data to make your life easier. In a databse design you also would have tables for a character, a position table, an inventory table, an item table and so on. The reason why you do this is that your SQL Queries are later easy and sane. And you can easily add new things. But what you are currently doing is just building a single table that contains "everything". You can do that, but this way it will be harder to work with, not easier. Putting things into Roles don't really help at all. Yes some code is in other files, you don't have a "big class", but you don't get any other benefit out of it.
Again, if I find I need to reuse this behavior, maybe I'll go that road, but it would be a role, not a class. Making classes for no other purpose than saying "I have something to delegate to" doesn't make any sense. Instead, we're talking about shared behavior and that's what roles were created for:
And they still don't have the benefit of just the creation of a sepearte class. In my Part III i already describe some benefits from switching from Roles to Classes. And overall, it makes code simpler, easier with a better code-reuse.
Actually the idea behind Composition over Inheritance also applies to Roles.
You don't want to make the naïve mistake of trying to mirror them in your software (you only create functionality you actually need and that fits your problem domain),
Sure, but using classes instead of roles doesn't mean you do that.
You don't make "movement" a class. "Movement" is a behavior with some state attached to it.
Sure, you do that. That is exactly what you call "Composition over Inheritance". http://en.wikipedia.org/wiki/Compositionoverinheritance The Wikipedia example even nearly applies to your code.
The idea that a class only should represent abstract things like "Customer", "People", "Car" or other things is not really right. A class represents something specific. In a full object oriented language even something simple like an "int", "float", "string" is already represented as a class. A class should represent one specific thing, it doesn't matter how abstract it is.
And more complex classes are build in re-using existing classes. I mean you also probably use things like Arrays, Hashes, Strings, numbers all of that are typcial things that are represented as a single class in a language. Those are buildings blocks to build more abstract classes.
A simple float is just a float. Put thee floats together and you have a 3D Vector. You usally want methods like calculating the distance betwenn two Vector3, calculate the magnitude or add/multiple two vector3 together. So you create a class for that.
Your character should have a movement. A movement needs a current Position that gets represented as a Vector3 and a target Position that is represented as a Vector3. And you usally want some algorithm to calculate a path from source to target. So a Movement class uses two Vector3 objects and adds methods to calculate the path from source to target.
A Character should move so he gets a Movement object that does exactly that for you.
I mean, if you want a string, number, array or hash you also never inherit from an array or inject an "array role" in a class, or do you?
Also, while your comments about the single responsibility principle are good for the theoretical case, they often break down in the real world, as explained on the C2 wiki.
Well, nothing is perfect, but could you provide the exact link to the example what you meant again? I think Movable Type has eaten the Link. It gets highlighted as a link but it has no "href" attribute.
Movement is a single responsibility, so it should be its own class.
No. A movement is an action. It's an abstract thing which doesn't stand on its own, like a character or a space station. It has to be attached to something concrete to exist.
The idea that a class only should represent abstract things like "Customer", "People", "Car" or other things is not really right. A class represents something specific. In a full object oriented language even something simple like an "int", "float", "string" is already represented as a class. A class should represent one specific thing, it doesn't matter how abstract it is.
Creating classes just to say "I have a class" is bad practice. If the thing I'm modeling can't exist on its own, it shouldn't be its own class. An Int, a Float, or a Char can exist on its own, so it's OK to model them as classes. That being said, in this context they can only exist on their own in relation to their problem domain: a program. In the real world, there are no "numbers". Those are merely convenient shortcuts we use to convey information. So it's all context dependent and as I've said, "movement" requires something else and doesn't exist independently. For most cases, it's probably a cross-cutting concern that properly belongs in a role. In my case, it's intrinsically part of the character class and will only be shared when I have that problem. YAGNI.
The benefit of it is that you create small classes that are easy to understand. And because it is a class you also can better test them. Actually, you can't really test a Role.
I test roles all the time. They're trivial to test. You create a stub class in your test which consumes the role and write your tests. It's trivial and let's you focus exactly on what the role needs. It's easier than testing the role via a regular class which consumes it.
And this is the URL you requested. Not sure what happened: http://c2.com/cgi/wiki?OneResponsibilityRule
No. A movement is an action. It's an abstract thing which doesn't stand on its own, like a character or a space station. It has to be attached to something concrete to exist.
The idea that something has to be stand on its own absolutly is wrong. And additionally a Movement stand on its own, a number does not.
A number on its own has no meaning at all. You always give meaning to a number. For example you can say a number represents "meters" or "miles". Just "5" on its own does absolutly represents nothing. And we still use int, float etc. as full classes. Because their just have exactly one Responsibility. And that is, handling a number.
A Movement can absolutly stand on its own. A Movement consist of a current Position and a Target Position. "Moving" means "Changing the current Position". There is absolutly no relation at all between a character and a Movement. A Movement never needs to access the missions, the inventory, the name or any other information that are related to a character. A Movement also never cares what exactly it moves.
The only thing that a movement cares about is its own Position. You usally use the Movement to change the Position of Something. What is Something? It doesn't matter at all. It could be your character, it can be a car, a bird, boat. That is often represented as Movement. Where T can be anything, it doesn't matter what it is. And what you have here is exactly such an design. It can even be that T is nothing. For example you provide just a Source Position and a target Position and the Movement just calculates a Path (That is an array of Positions). You use this to represents a line to visualize movement of enemies (for example in a tower defense game).
I test roles all the time. They're trivial to test. You create a stub class in your test which consumes the role and write your tests. It's trivial and let's you focus exactly on what the role needs. It's easier than testing the role via a regular class which consumes it.
You still can't test roles on its own. You use a workaround to test a role. And that even makes no sense to me at all. So what you are doing is.
1) Creating a Role for a Behaviour 2) Creating a stub class that just consumes the Role 3) Test the stub class
If you create a class from the beginnging you don't even need a stub class because you already have a class: What you end with is just:
1) Create a class for a Behviour 2) Test that class
Actually what you do currently is even more complex. So for every role you anyway create a class. And by the way. If you can create a stub class that just can consum a single role. Than you also have proven that a role can stand on its own.
And this is the URL you requested. Not sure what happened: http://c2.com/cgi/wiki?OneResponsibilityRule
Okay, but i don't really say where people argue that is bad. Actually most of the comments argute that you should follow that principle. There is one person that criticize it would be unrealistic for "real-world" application. But you have even more people that try to prove this as wrong.
One explanation that i like for example
Perhaps, TopMind, you should explain the situation you are imagining. In practice, 'nouns' are things like 'connection', 'queue', 'stack', 'functor object', 'input stream', 'output stream', 'unum factory', etc.. These nouns are distinguished by interface and contracts on behavior - by responsibility. You can learn much about the 'nouns' used "in practice" by researching WikiWiki and SoftwareDesignPatterns. It is true that there are relationships between objects... e.g. one might create a message stream atop an octet stream, which is a 'uses' relationship. The OneResponsibilityRule favors constructing objects that perform delegation, such that the message stream is ignorant of what happens after it flushes the octet stream. (i.e. "You give me a tube. I break these messages into bits and put the bits in that tube. What happens after that is not my responsibility.") If there are many<->many relationships, then those relationships tend to be encapsulated by behavior and responsibility of dedicated objects... e.g. using 'signals' in EventDrivenProgramming is primarily a pattern on many<->many relationships, and is a common SoftwareDesignPattern for
Actually that is exactly what i suggest you. You have Movement. What does Movement? It calculates a Path from Source to a specified target. After the path is calculated Movement change its current position. And usally you have a lot of ways how you can do it. A Time based movement. Move from A to B in a specific time. Do you want Lerping. That means slowly start, slowly end. How fast is the position changing? An Car should move faster than a character. So your Movement have a Speed variable and so on.
Actually you can even have different implementations of movement, with the same interface. For example a movement under water should be another on land. So you can have different classes like
EarthMovement WaterMovement AirMovement
And you just can swap the Movement class if you wish. Your character is under water and now has other options to move (up/down) and another speed. Easy.
$character->movement(WaterMovement->new( currentposition => $character->movement->currentposition, ));
And voila, you have completly swaped the behaviour at runtime. Now try this with a role.
Nah, some characters and formating has gone wrong. It's sad that MovableType has no way of edit a comment. But another thing what i wanted to say. You mentioned YAGNI.
Actually what i say doesn't invalidated YAGNI at all. YAGNI says that you only should implement what you need. If you don't need movement, then you also should not implement it.
But if you need Movement, then you implement it. YAGNI doesn't say: "Hey, put everything into one giant class".
I think what you see mean is: This doesn't mean you should avoid building flexibility into your code. It means you shouldn't overengineer something based on what you think you might need later on.
Especial the last sentence. But putting everything into a seperate class doesn't validate that. Putting it into another class instead of an already existing class is just a design decision.
It gives you more flexibility later on. That is why i suggest it, but it is still not what the last sentence says.
YAGNI would be if you also would implement things like Flying or Swiming in Movement, even if you don't need it at all when you create your movement class.
That you can re-use it is something that you just get "for free" designing it that way, that has nearly zero overhead. Especially not if you anway create stub classes for testing.