I've wanted to be a professional game programmer since I was in high-school, back when we all still had to park our triceratopses (triceratopi?) at the trike-rack outside the gymnasium. Over the years I've always considered it to be my long-term goal, but it seems that life sometimes does not take these things into consideration when handing out circumstances (it seems to pay far more attention to what I do rather than what I want). That being said, I'm at the point now where I think that if I keep waiting for a golden opportunity to arrive then it simply never will.

I've thus decided to strike out on my own and just start building a portfolio of projects that I can show to potential employers. The thing that bothers me is that I don't trust my own judgement as far as how "good" my work is, and since that concern centers around my technique I don't know how to ask what I don't know about - apart from showing an example of my work and asking for a critique from people with professional experience and perspective.

So what I'm asking, and I apologize if this isn't the correct forum section, is for some folks to take a look at my first portfolio project and let me know about where my code stands in relation to what's expected for a professional portfolio. I did study some programming in college, but it was a long time ago and frankly the particular courses I took were a waste of money. I'd certainly like to go back to school, but I've been trying for a decade to put together the resources for that and I just don't think that it's going to happen, so I'd really like to have an impressive portfolio to show.

So my concerns are:
* Are there any problems with my code/style/method that I need to work on?
* Does my code demonstrate a lack of accepted technique in any area?
* Are there any smells that stand out?
* Is there anything here that would be likely to make a potential employer unimpressed?
* Is there anything which, by its not being here, would make a potential employer unimpressed?
* Is there anything that's not here that would make a potential employer impressed?

A general review along that theme is what I'm asking for. I do realize that it's a pain to crawl through someone's code, and I'm certainly not asking for a comprehensive study or anything. I'm just humbly requesting that if any folks here with professional experience could give it a once over and let me know if I'm on the right track and if not then let me know where I'm going wrong.

Any advice would be greatly appreciated, including advice about where I can get this kind of advice. I'm really pouring myself into this. I know in my gut that I'm a programmer and I'm sick of living as everything but. I'm ready to do what it takes.

This is the Visual Studio project containing the code and resources in a rar archive.

[attachment=10098:Bricks.rar]

The project was designed under VS2008 and relies on 2 external libraries for compilation:

DirectX SDK(using DX9c) - http://msdn.microsof...x/aa937788.aspx
irrKlang(ver 1.0.3b) - http://www.ambiera.com/irrklang

Thanks in advance for any response.

I've only just quickly looked through your code and the first thing I noticed and what I think you really should adept to is naming your variables. Or rather, clarifying what these variables are. If I should work with your project, there is no way for me to quickly know whether a variable is a member of a class, a passed argument, const, local, etc etc. This is not mandatory for your code to work obviously, but its better to get used to a general best practice when it comes to that.

For example, all my member variables start with "m_" (m_Ball, m_Wall) allowing me and others to instantly know they work with a member variable. I use "a_" for passed arguments (void magic(float a_Number)) etc. In general just let people know with what kind of variable they are working with so they don't have to look it up.

Another word of advice: Employers, if they would want to play the game at all, will generally not compile your game. Include a working .exe so they can play, or if you are allowed to show it at an interview, you don't have to compile it. Should an employer even go as far as to willing to compile your project, they will surely not download any packages and link them to do so, include the needed headers in your project. This will obviously expand your project size, but might be worth it. Depends on the tradeoff I guess!

Hope it helps!

Drive by thoughts:

• RAR is less accessible than ZIP. Everybody and their grandma can open a ZIP file, RAR is just a slightly higher barrier to entry. Prefer ZIP.
  
• Your README is informative but unprofessional (I'm thinking of the "Legal Junk" section here). Consider polishing it since that's the first impression I got after opening your archive
  
• Code is generally readable and fairly clear, so that's good!
  
• Unfortunately, you seem to lack a consistent naming convention and formatting convention. This is a major negative.
  
• Your usage of exceptions seems suspect. I didn't have time to really dig into it, but it seems like you're not familiar with proper exception safety techniques in C++. I'd suggest brushing up on this.
  
• Getters and setters with no extra logic are waste. Just use public member variables.
  
• I see lots of magic numbers, which worries me. Try to use descriptive constants instead whenever possible, and comment any other magic numbers you have to use.
  
• I'm curious why you chose to use a lot of dynamic allocations (e.g. for every brick in the game) instead of something more compact or efficient. Obviously compactness and efficiency aren't everything, especially when you just want to get a project done, but if you want to work in the industry, you're going to have to get used to justifying every single allocation you ever make :-)
  
• You seem to sprinkle a lot of Microsoft/Windows-specific stuff in with little consistency. Why UINT instead of unsigned? This drops your portability a bit and makes your code harder to look at. LPSTR is also a bad sign, for example. Cleaning this stuff up would probably let you port your game much faster, which shows you understand platform-agnostic design and implementation - a major thing in many studios.
  
• Mixture of C and C++ is bad. For example, why aren't you using C++ strings? Why memcpy instead of std::copy? etc. Learn the standard C++ library and know when to make use of it.
  
  That's just a few things, but hopefully it's enough to get you started (and maybe keep you busy for a while!). Best of luck, and I look forward to seeing your next pass! :-)

My comment is basically a mirror of ApochPiQ's comments.

Zip file, your readme needs help, provide an executable and a link to a youtube video.


Overall I'd say it is good for breaking in. It shows that you can program. It also shows a lot of beginner errors, such as extensive magic numbers and inconsistent naming.

The mistakes you make give me a better assessment of your programming skill. For example it is clear you have either a reluctance or ignorance of the standard library, I'm guessing it is ignorance because you don't have experience. The try/catch/catch/catch/catch clauses you have peppered through your code shows that you likely either came from Java or have no idea how exceptions should work in c++, or both. All the member pointers and allocations during construction rather than RAII also indicates a java background. The lack of useful comments shows you have no real experience with long-term code.

In the files I looked it, this comment jumped out at me enough to point it out. It says you would rather add a hack than fix the bug:
//This 'else' may seem unnecessary, but if the disp is
//somehow higher than the score it will correct that.
//(true story...)

This line from your readme is mildly disturbing. You say you recognize the copyrights and proceed to intentionally violate them:
Some of the audio and texture resources in this project were not
created by the project author. The project author recognizes the
copyrights of the respective owners. Any copyrighted material
will be removed upon request by the owner.


It is about on par with what I would expect for a new hire fresh from college.

It is not up to par for a code sample of an experienced professional.

Good luck on your job hunt!

Thank you, guys! This is exactly what I'm looking for.

I am indeed new to exceptions. This project is actually the first time I've used them, having always used return values before. I'll definitely seek out some more information about using exceptions in C++. I'm really not happy with the current error management strategy in that project, for obvious reasons, but I can sort of sense that a better methodology could be worked out and I'm interested to see how other people have worked with it.

I actually do use a sort of naming convention, but it's complicated to the extent that I wouldn't expect other people to recognize it. I'd probably be better off shifting to something easier for other people to understand.

With the magic numbers, are you referring to the slew of literals I fed into window creation for styles? (I guess I can just search the solution for numeric literals. I really should do better with that.)

As for the comment about that 'else' statement, I believe that the 'else' was the correct solution to the logic there. The comment was to remind me not to remove it, since a couple times I browsed through there and thought for a moment that it was redundant. EDIT: Correction - I remember now that I also fixed the bug, which was an incorrectly initialized variable, but thought it prudent to leave in the safety mechanism since it completed the logic. Do you think I should remove it?

RAII is something I haven't heard of before. I'm looking over the wiki article now. Thank you. I considered a retention strategy for the brick objects to avoid allocation but thought that it would be sloppy and possibly lead to errors. If you say that dynamic allocations are not to be preferred then I'll certainly take another look at it and look over the code for allocations that can be avoided.

As far as STL I've been reluctant to use it in the past since the angle bracket syntax and the iterator methodology really put me off, but after having read some literature about it and understanding that it's an accepted and well used system I've started trying to make use of it. I'm working to improve in that area.

It's funny that you should mention Java. I actually downloaded the JDK shortly after posting this thread and just started reading over the Java documentation this week. (I thought that was kind of funny. Could just be me. ;)) When I originally decided to try to teach myself to program a few years back I started with Ruby and from there dug out my ancient and dusty C textbook from college. I later moved into C++ and briefly looked at Lua (Which I didn't really like, having been spoiled rotten by Ruby.).

Concerning the getters and setters sans-logic, I've read in a lot of places that public member variables are bad and some people flip out about it. I'd really actually prefer to use member variables in several places. Have I heard wrong? (I hope.)

Concerning the copyrighted material, those resources are all things that are either licensed for re-use (the sfx and some of the textures) or else freely available for download. Really the only reason I'm using them is that I don't have anything else appropriate to 'fill in the blank' for now since I'm a horrible artist and so tone deaf that I actually get disability pay for it (j/k). The only one that's really questionable in my mind is the soundtrack for the hi score screen. You're right, though. I'd rather not have to put that clause in at all. I'll look around for some resources that I don't have to worry about.

I didn't even really think about trying to making the readme file more professional. I was just putting that in so people would be able to compile. It's a good point, though.

As for the commenting, I've sort of been kicking myself this week for not thoroughly commenting Ball::doMotion(). I try to make the code 'tell its own story' in most cases. I think what I really need here is a bit more perspective about what I should and shouldn't comment. I'll look over the solution again and try to think harder about it from the perspective of someone new to the code.

Finally, concerning the use of the MS type names, I hear what you say. In the past I've tended to use my own typedefs, such as u8 rather than BYTE or etc. It just seemed that since I was developing in a Windows environment it would probably be bad to mix the two by spitting out a bunch of my own typedefs to add to the MS ones, especially since the MS ones are needed as arguments for a lot of the DirectX functions. It's occurred to me that I may need stronger decoupling between the engine and the game-proper and this may well be a smell resulting from that problem.

Anyway, that's a good bit of advice and actually a lot more encouraging than what I was expecting. Thank you all very much!