Keeping methods readable and concise

Also clean code in my view is kept for the domain. The close to the metal code that supports the domain sometimes need to look ugly. Like this code I wrote today at my dayjob. Here is the close to the metal code

Code (CSharp):

1.         private async Task ExecuteCommand(CommandInfo dbCmd)
2.         {
3.             int id = 0;
4.             using(var scope = serviceProvider.CreateScope())
5.             {
6.                 var ctx = scope.ServiceProvider.GetService<IBusinessContextController>();
7.                 var repository = scope.ServiceProvider.GetService<ICommandRepository>();
8.  
9.                 try
10.                 {
11.                     var cmd = JObject.Parse(dbCmd.Json).ToObject(Type.GetType(dbCmd.Type)) as Command;
12.                     cmd.Id = id = dbCmd.Id;
13.                     var handler = serviceProvider.GetService(GetHandlerType(cmd.GetType())) as dynamic;
14.                     await handler.Handle(cmd as dynamic);
15.                     await repository.UpdateStateAsync(cmd.Id, CommandState.Completed);
16.  
17.                     await ctx.SaveChangesAsync();
18.                     ctx.CommitTransaction();
19.                 }
20.                 catch (Exception e)
21.                 {
22.                     //TODO: log e
23.                     ctx.RollbackTransaction();
24.                     await repository.UpdateStateAsync(id, CommandState.Error);
25.                     await ctx.SaveChangesAsync();
26.                 }
27.             }
28.         }

But the domain code will be very clean and reliable

Code (CSharp):

1.         public class MyTestCommandHandler : ICommandHandler<MyTestCommand>
2.         {
3.             private readonly ICqsClient _client;
4.  
5.             public MyTestCommandHandler(ICqsClient client)
6.             {
7.                 _client = client;
8.             }
9.  
10.             public Task Handle(MyTestCommand command)
11.             {
12.                 return _client.AddCommandAsync(new MyTestCommand());
13.             }
14.         }

Above is actually the core foundation for a new enterprise system I'm writing

 

How is that specific to game developers?

 

zero difference, more game devs should maintain their code bases as a business app.

 

This got a ton better with C# 7, where you can have nested methods:

Code (csharp):

1. void Method() {
2.     DoThing1();
3.     DoThing2();
4.     DoThing3();
5.  
6.     // helpers:
7.  
8.     void DoThing1() { ... }
9.     void DoThing2() { ... }
10.     void DoThing3() { ... }
11. }

In the majority of cases where I used to split out parts of functions into a different, private function, I now just turn them into local functions. This helps keep down the number of methods the class has, which makes it easier to understand - I like looking at the structure tab in my editor when I try to understand a type, and if that's littered with a bunch of private helper methods, it becomes hard to see what the type does. When the methods are local, that's not a problem anymore.

I also find that when you split out a bunch of private helper methods, they tend to drift away from the method they "belong to", as new methods are added to the class, which also makes the class harder to read. When the methods are local, they don't have that problem to the same degree, as they are within the scope of their owner method.

 

You didn't ask for a code review, @AndersMalmgren, but

Why are you using safe casts here? Wouldn't it be much better to get a ClassCastException if your assumption about eg. what JOBject.Parse returns than to get a NullReferenceException later?

 

It can never happen because I enforce the type

Code (CSharp):

1.     public interface ICommandHandler<in TCommand> where TCommand : Command
2.     {
3.         Task Handle(TCommand command);
4.     }

But even if it did happen it would mean a crash just a line later when I use the instance

 

An interesting read through. I always find these discussions kind of humorous because ultimately as @Kiwasi kind of covered... yep everyone is right.

There are many books on how to cook many different approaches for something as simple as cooking an egg even. Each person believes their way is the one right way... and of course it is true or half true anyway... it is the right way for them. For someone else a different way may be right.

Ultimately it all comes down to focusing on which way is best for you personally. Use what works for you and don't use what doesn't. Forcing use of things that don't naturally fit you just adds complexity & confusion and in doing so largely defeats the purpose of using them to begin with. Simpler is generally always better but again simple is a very vague thing. Clean is a very vague thing.

Some would consider explicitly specifying concrete types for variables as being cleaner more readable than using var while for others operating in an environment where devs all understand the type that is in use through the local context var is cleaner more readable. Again these are very personal things along with how do you crack an egg the big end, the small end or in the middle? Answer... depends on who is cracking it.

 

That's awesome, I didn't realize that was available. I'll make use of that for sure.

 

True, but if your domains readability is dependant on explicit type declaration then you have deeper clean code issues. Which is my point

 

I want MS rename the 'var' keyword to 'war'. Because it is. (BTW, in strictly typed languages it's dumb to hide the type. No matter what. In Javascript it's completely okay to have 'var', in C# it is not. Uff.)

Also, we really should stop to talk in generals. All the effort you put into your code should depend on the expectations. If your cost-benefit analysis says that you should build the biggest MVVC framework imaginable for your game, then do it, if it's temporary or you're doing something quick and dirty, then be it. I don't like faith-based decision making.

Also 'clean code' is a vague term. Clean means different things for different people and different teams. Find your style, your code and make your own compromises if needed.

Also 'dirty' code is wrong only if it really hinders your (the team's) performance. Otherwise, nobody cares. If you develop your own thing and you don't release the code publicly and you feel comfortable, then just work. You shouldn't care about anyone else's opinion about how clean your code is. Develop yourself and your knowledge about these things is okay, but wasting your time on unnecessary refactors and headaches to re-architecture your code is not a good decision.

 

So many things wrong with this post. First, even if it will stay a one man team all through the life time of the project maintainability is also a thing in a one man team. Second, you might hit a success and need to expand. Maintainability is always important. I guess if you are a complete noob its not relevant. But if you start to get used to programming you should understand the different trades of a maintainability code base.

And lastly, complete nonse about C#. C# is a static typed language. It gives you some compile time check and sures. Doesnt mean that the types are a holy grail and are super important, the domain is whats super important. C# isnt any less static typed with vars

 

Have you never created a project to replicate a bug to send in for a report? Because that's the first thing that comes to me when they say "temporary". "Quick and dirty" immediately made me think of the little prototypes I build when I want to test out an idea. Game jams would qualify too since they're basically just prototype contests.

 

Sure and that domain is reproducing a bug, in a readable manner so it's easy for the team to understand.

 

I don't think it's a mystery at all. I'm very capable of writing good clean code. I often don't because the game design process is so organic. A lot of the time you don't know exactly what you're building until it's fun and you don't know if it's fun until you've stumbled across it.

Personally I don't like to invest in over-engineering for all possible gameplay permutations so I simply hack stuff together. I think that unblocking the design process is more important to a game and project than keeping the codebase as clean as can be. My reason for this is simple: Reduction of wasted effort. I'm not going to spend time building clean systems that I may just throw away. Also, if a system is ugly then I'm less likely to care if I need to throw it out!

That said there comes a time when you have settled on a feature set and at some point you'll benefit greatly from cleaning things up. Again I treat this as an organic process, kind of like cleaning my room. When the effort involved in cleaning my room becomes less than the effort I expend looking for things in my messy room, it's time to clean it.

Edit:
That analogy I just gave about messy rooms is a good one actually. A messy room is ok if it's your room, but if you're in a shared house, messing up the kitchen is not ok. I guess that's similar to a team of programmers on a project. Mess up the thing that only you touch, but if it's shared there are standards that have to be kept. You should probably also setup a cleaning roster too.

 

Well, I basically work the same, though I always have some maintainability always at my mind. But in POC state I dont care much, dont even care about allocation or whatnot.

edit: But that POC is easy to test because of a solid foundation. And when its ready I can refactor it easy

also todo comments are good, then the IDE will help you clean it up

Code (CSharp):

1. var sorted = hits.OrderBy(h => h.DistanceTravelled).ToList(); //TODO: Linq for now

 

Great point. "Enterprise" type code (I'm using that term very loosely!) is often written to strict requirements well known up front, or closely maintained via some kind of strict change control system. Often, neither of those things apply to game code until rather late in a project, if at all.

(Emphasis mine.) Even I don't agree with this.

Ever worked with multiple layers of templated types in C++? You can get some super verbose type names under those circumstances, and using "auto" can indeed make things far easier to understand.

It's also handy in cases where you might want to allow the underlying types of some things to be able to change without necessitating modifications to other source files. With "auto" or "var" the compiler will just deal with those when it runs into them, where with explicit type names a human being has to go to various source files and replace one type name with another.

In both cases you re sacrificing code clarity, because you're adding steps for a programmer to understand what the code is doing. The idea, though, is that you're sacrificing clarity of details which shouldn't matter, or which should be handled elsewhere, while saving some effort and hopefully also making the client code generally easier to digest.

One of the major difference between "enterprise" developers and game developers is that with enterprise stuff it's taught and it's expected that details can be left to the computer or compiler, where with games it's generally the opposite (and sometimes too far, in my opinion).

 

If you need the type to make the code understandable you have other problems with your domain

 

That's a perfectly reasonable point of view from a high-level (of abstraction) point of view. From a low level (of abstraction) perspective the opposite is perfectly reasonable.

 

If you really need to see the type you can Hover the var symbol. Happens to me once a year, tops

 

btw, all the shortcuts like goto defintion, etc,etc works.. Even highligting the var will highlight all usages of that type

 

Using var is so 2010 UnityScript. You always throw out code that is not based on the Unity SDK.. In these cases i say..so what.. you are simply bringing your business domain habits into the Unity domain. I have been developing in Unity for 9 years and your senior dev hype may merit cracking a beer in your business work but your code is opaque and has nothing to do with Unity's SDK.

 

No they shouldn't. I saw this very mistake at my last contract. They are now swimming in a never ending beta and i quit. This is not a C# business domain. It is C# as a scripting language for a frame dependent, component based, real-time interactive 3D architecture. Ergo it should be handled as though it is in that domain..i.e. use the Unity SDK. If you know how to use that domain correctly then monstrosities like your weapons trigger pull and handling system can be authored as a few relatively simple drag and drop components. Something I note that C# business devs have a seriously hard time wrapping their heads around until one day when the light goes on.

 

haha, that trigger system is crazy modular and dynamic. It can control 5 fingers at the same time operating different parts of the item without the animation breaking. For example, how can you solve this with classic unity animations; When I cock the hammer the trigger finger stays on target even though the hand moves away.

edit: And all that in just a few lines of code in the core domain

Code (CSharp):

1.                 if (!HammerCocked && AttachedHand.CheckForPlayerCommand(Command.CockHammer))
2.                 {
3.                     AttachedHand.SetFingerIKState(Fingers.Thumb, true, 0.05f);
4.                     NetworkedFirearm.RequestManualCockHammer();
5.                 }
6.  
7.                 if (AttachedHand.CheckForPlayerCommandTouched(Command.CockHammer))
8.                 {
9.                     AttachedHand.SetFingerIKState(Fingers.Thumb, true, 0.25f);
10.                 }
11.                 else if(!ownerUncocking)
12.                     AttachedHand.SetFingerIKState(Fingers.Thumb, false, 0.2f);

 

@ippdev Any input?

 

Yeah. Yer fallacy is I would use classic animation. I would use perhaps three separate animations and blend trees. (Unless I was doing cowboy twirls and unholster/reholster revolver handing). This would provide me every hand position I needed to handle a pistol of any kind. Simplifying trigger pulls and controlling trigger finger positioning would be handled by placing the finger IK Goal on the trigger of each pistol. The finger wouldn't pull the trigger. The trigger would pull the finger..saves alot of variable lookup and calcs. Would have had a universal system written in a couple of components in a few days including testing and precision set-up rules for pistol trigger IK goals. It's yer game..knock yerself out of you want. It looks good but you should frankly have a helluvalot more completed IMHO. And that in a nutshell was why I quit the business coder gig. They were running in effing circles to conform to some MSDN dev BS and making things infinitely more complex than needed and not getting dick on a stick interesting or challenging done at all. I Live to create..not collect a paycheque. And they did not understand the Unity SDK and kept writing crap that was an overlay to the Unity SDK thinking their little habits jammed into the framework were proof of their intelligence. Each little addition kept dropping the FPS as well after I took it from 10fps to 180-270 fps on same models.. When I left it was getting 50-60. Five months later and still in beta and I know I was correct. The VP agreed with me when i spoke last week with him.

 

The problem with that point of view is that the domains have different goals. With enterprise business software, the main goals are to get the right answer and implement it with the smallest number of programmer hours. There is usually very little concern for performance in enterprise business software. In many enterprise situations, the company owns and controls the computer hardware that will run the enterprise business software. The company can easily buy more powerful computer servers to run the enterprise business software, and that usually costs significantly less than paying programmers to optimize the performance.

With game software development, performance is usually one of the absolute highest priorities. It often makes sense to build custom code that cannot be modified easily or re-used easily, because that code is very performant in its specific use case. While an enterprise business developer is usually more concerned about how quickly the code could be modified when middle management makes requirements changes mid project, a game software developer is more concerned with how many milliseconds each function needs per frame.

As a thought exercise, think about the different ways you could approach a specific programming task. For example, let's say a game needed to display the top 5 scores of currently connected players on screen. An enterprise business software developer will come up with a solution that displays the correct information. An experienced game software developer will take longer to implement it, but will come up with code that is faster though harder to modify in the future. If/When the managers decide in the future that the top 5 should actually be a top 10, the enterprise business software developer will easily make the change in minutes. The game software developer will take longer to make the change, but again the game software developer will implement a custom solution that is measurably faster.

At this point you will probably argue that the enterprise business software developer is clearly better, because the performance is not that big of deal. After all, modern computers are fast, and top 5 score board refreshes only need to happen when a player's score changes. But all of the little optimizations do matter, and they do add up. By the end of a project, the enterprise business software developer approach can easily require twice as many milliseconds per frame as the experienced game software developer approach. That means the experienced game software developer approach can run twice the frame rate on the same computer hardware, or the same frame rate on half the hardware. That is important, because in game development, the end users (instead of the company) controls the computer hardware purchasing decisions.

 

I write all of my code like the first example. What I dislike about sectioning off things with comments is that now the comments have to be maintained also. If they are not maintained, it is very easy to end up with misleading comments which is even worse than no comment at all.

The second way is not wrong it is actually better performance wise because the extra function calls do have a cost. But in my opinion those longer functions are harder to work with if you expect to change them at least 50x which is usually what happens to me as we go through test iterate, test iterate cycle. So I always do it the first way. Because I prefer easy to change. And I always follow the Single Responsibility principle

 

Yes I agree. Great Suggestion. Switches are a very bad choice if you want a complex weapon system with 300 different weapon types and new ones being added regularly. The code will become un-managable without a proper Object Oriented solution such as what Tony suggested. Interfaces will do the trick also.

 

Thanks. I just want to reiterate that I'm not advocating a specific solution, but a mindset. If code feels clunky, then when you refactor it, step back and consider if there are other approaches that will yield a more elegant result.

 

We tried having a generic solution with the trigger driving the IK, but it didnt work. The physical difference of trigger locations relative to hand, the muscles in your finger and combined with the friction of your skin is hard to mimic generic, didnt look good. So we ended up with key frames for the IK ontop of the trigger movement.

Sure it could have been solved with unity anim controllers and blend trees, but not sure it would have been any less complex. Plus this way we have a really flexible way for rigging, my wife is doing the rigging and she is a very good artist but none technical. She does all the rigging inside unity in a tool I made for her

Plus we talk around 300 rigging at this time. And it will be more. Good tooling is important.

Also like I said a complex state machine, imagine doing it with mecanim animator controller and blend trees, for example combining the hammer cocking, with trigger safty

When you count in all special cases I'm not so sure your way will be easier