Full Disclosure
I am not a hiring manager, nor do I routinely vet candidates for positions. Everything I say here is my own opinion and has nothing to do with any of my employers past, present, or future. Also, if you misuse anything I say, I will kick a penguin.


Advance Apologies
I'm gonna be brutal. Get your flame-resistant gear on. This isn't intended to tear apart all your work; think of it more like tough love. But it'll probably sting [smile]


First impressions

• Web site design is decent, but the backdrop is a bit overwhelming. Maybe I'm just channeling a graphic artist here, but IMO there's room for improvement. But that's not too terribly important; you're not advertising your web design skills [wink]
  


• One of the first things I saw was the phrase "code and stuff!" - this strikes me as a little too casual. I'd suggest staying fairly formal in your portfolio work, much as you would in a resume/CV. I sincerely hope that nobody in a hiring position out there would dismiss you over such a tiny thing, but why give them any ammunition?
  


• I clicked on your Work page and was a little confused at the lack of detail. It took me a couple of minutes of poking around the site to realize that the sections can be expanded. I would suggest just keeping them expanded all the time, so nobody gets confused as to where the content is.
  


• What's "post-process programming"? I can think of several ways to interpret that phrase. A little more clarity there would go a long ways, I think.
  


• Again a few bits and pieces here and there could stand a proofreading pass and a little bit more formality. For instance, using a bulleted list on the points you've worked on would help a lot with making the page both attractive and accessible.
  


• Also, "I cover pretty much every base(codewise) in our production to create a game we know players will love." This sentence is just kind of unpleasantly sticky and has a faint aroma of rotting fruits. Be clear and specific about what exactly "every base codewise" really means. From one game to another there can be massive differences in what you actually work on.
  


• "Also, make sure to look at my 'work' page too!" If anyone is scouring your portfolio, they're probably going to click on every link they find that seems relevant. I don't see the purpose of corny little reminders like this.
  


• While I'm clicking around, the division of "Work/Games/Tech" seems really arbitrary and doesn't make much sense to me. Consider reorganizing things a bit. Once again, clarity and simplicity will take you far.
  

Code Review
I'll keep this brief (for both my sake and yours) and to the point. I randomly picked SHRPskeleton.h/.cpp from your code sample archive, and these are my impressions.

• Your commenting style is... not good. It's a massive waste of space, for one thing; it's also prone to get out of date and become inaccurate - and nothing is more evil than inaccurate commenting. Repeating information (file names, class names) is bad. Explicitly spelling out the purpose of every file and function is redundant; you should find that only a small subset of your code actually needs to be commented, and the rest should be self-explanatory. Tracking authorship and dates is pointless, as that information can be had from your version control software (you do use version control, right?) in the form of revision histories and "blame" tools.
  


• In the SHRPskeleton class, you have an extraneous access control specifiers, namely the private/protected pair on lines 36-37. This indicates to me that you don't routinely recheck your code for things that can be cleaned up.
  


• When working with function pointers, I highly recommend you use typedefs instead of spelling out the entire function signature everywhere it is used. This makes maintaining the code much simpler.
  


• Why is m_Skeleton public? Why are you using an m_ wart that apparently doesn't do what most people expect it to do, i.e. signal a private/protected member? Either this is a mistake (and should have been caught by a routine code review) or it needs to be documented. Also, warts are evil.
  


• FooFunction(void) is not preferred C++ syntax; just use FooFunction() instead. This is a bit of a stylistic debate, but most code standards I've seen lean towards not using (void) parameter lists.
  


• I don't like the habit of not providing parameter names in your function prototypes. I would like to be able to pop open your header file and read the interface, and know what a given function does, just by looking at the declaration. I don't want to have to pop open the .cpp file just to figure out what arguments a function expects from me.
  


• References are your friends. You should prefer references over pointers in just about every case where they are used in the code currently.
  


• Why do you define a destructor for the class, and then leave it empty? Unless you need a virtual destructor for a base class, just leave out the destructor entirely.
  


• Eww, const char* parameters! Please give me a good justification for not using std::string instead.
  


• C-style casts (such as (int)(foo) and so on) are very, very, very bad. Use C++ casts: static_cast, dynamic_cast, reinterpret_cast, and, if you are living life on the edge and doing something which is probably unwise, const_cast.
  


• void pointers are evilness incarnate. Unless you have an extremely good excuse, I would say that void pointers are a strong indicator that your code architecture is broken.
  


• Mixing class member functions and local functions is icky. Please keep them separate, preferably containing local functions in an anonymous namespace so as to avoid linkage issues.
  


• "I'm so cool/I blew my brains out thinking about how this works" - just... no. Maybe you can get away with funny comments once you've been established someplace for a while and people trust you; but if you're looking to make a good, professional impression, hold back on the humor - and the hubris.
  


• if(!foo.size()) is potentially an expensive operation for some containers. Prefer the form if(foo.empty()) instead.
  


• I haven't confirmed it, but I'd suspect you have a couple memory leaks lurking in there.

If you really want I can probably find more stuff to complain about, but that should at least get you going [smile]

I won't comment on the content, as I couldn't even be the judge of that. My experience does not lie within that area of expertise so anything I'd have to say would be irrelevant.

However, here are my 2 cents on presentation:
A lot of people will tell you to go straight-to-the-point, and you seem to do a good job at that. Everything seems in order for relatively quick to grab information, but there is clearly a lack of polish. Truth is, you're not mostly attempting to boast artistic skills here from what I understand, but it is my opinion that a bit of fleshing it up would add to the lot. As I've said, a lot of people want straight to the point with as little visual obstruction as possible. They consider it noise. But if you can expand on the visual without adding noise, then you're displaying personality, and to be bluntly honest, it is solely with personality that I got my first opportunity in the bizz. My cover letter was a disastrous poem but it turned out to bring out curiosity. I'll admit that was a LOT of noise, and a big risk to take, but there is such a thing as a middle ground.

If you can polish things up visually without altering the core, then I'd say that'd be a good improvement.

Then again, just my 2 cents.

Quote:Original post by StephenTC
If you don't know what post-process programming in relation to graphics is then you don't know what it is. That's your own problem. It's like asking a network programmer what a packet is.

I know what you were referring to, because I could infer it from the contents of the rest of your site. I'm not the one to worry about - you need to worry about the HR guys who aren't tech-savvy who have to vet your application. You need to worry about keeping your best skills in the spotlight, in this case, your graphics programming. You need to worry about showing that you are capable of communicating clearly and succinctly to people outside your domain of expertise. These are important qualities for an employee; technical skills aren't everything.

Also, keep in mind that using that tone during an interview is likely going to bite you [smile]

Quote:Right, I've heard this before, except that's how I had it and all everyone complained about was there being too much stuff on the page.

So add a "show more details" link or something. It certainly isn't obvious enough as-is.

Quote:*laughs* Indeed, it's PRish stuff. I'll make it a little more straight-forward. I literally mean everything, except for the core engine, which is of course Unity. If that sounds funny I'll clean it up.

List bullet points. People reading up on your skills love bullet points. Like I said before, "everything" can mean many different things depending on the type of game you're writing. Doing "everything" on a turn-based, hex-grid strategy game is a world apart from doing "everything" on a flight simulator with AI pilots. You need to be clear about what "everything" means to people who are not familiar with your project. More importantly, you need to be clear about what "everything" means to people who don't care about your project.

Quote:Old code, hate it myself. Taking it out.

Call me paranoid, but it's a major red flag to me that you've submitted a portfolio for critical review without first checking that you are happy with its contents. I certainly hope you don't have a similar habit for submitting code.

Quote:I do use standard libraries quite often, but only when I'm lazy and am just trying to prove a concept. The standard library is slow and chokes to death when compiled under debug. It's also not great when porting to different devices.

This attitude right here makes you an instant no-hire in my book.

The SC++L is there to make it easier to write safe, robust code. It's not merely a toy system that you can use for prototyping; it's a core feature of the C++ language itself. If you profess to having C++ skills, you had better know the standard library - and when I say you need to know it, I mean you need to really understand when and how to use the standard constructs.

We use SC++L code extensively in our projects, not because we are "lazy" or "just trying to prove a concept" - but because we know that using well-tested, standardized code is much safer than reinventing wheels all over the place.

As for your assertions about speed and debug builds... you'll have to cite some very convincing evidence to sway anyone on that topic. Suffice it to say that we've done heavy profiling and found the standard library to be a minimal overhead in a nontrivial game implementation. Yes, there are specific areas where it's best to use something lower-level, but those are very much the exception and not the norm.

Porting to "other" platforms is a specious argument at best. "Other" platforms are liable to have their own standard libraries, which can supplant the SC++L in many cases. The idea of utilizing any available code remains the same, even if you're talking about different libraries.

Quote:Same kind of problem with the above. Yes it works and yes it's somewhat safe to do what you say. This is a matter of opinion.

No, it has nothing to do with opinion.

struct Foo { int xorph; };struct Bar { float quux; };Foo* pfoo = new Foo;Bar* pbar = (Bar*)(pfoo);

Uh oh! We just did a very naughty cast using the C-style syntax.

Now if we replace the final line with Bar* pbar = static_cast<Bar*>(pfoo); we instantly trip a compiler error, noting that the two types are unrelated. We can use reinterpret_cast if we truly know that we want to do that conversion, but you risk wandering into the land of undefined behaviour. The C++ casts were specifically introduced to make casting safer and more robust; they are not "somewhat" safe - they are properly and completely safe, when used as directed by your favourite medical professional.

This wouldn't quite rate an instant no-hire from me, but it'd definitely be a major strike against you.

Quote:Oh yeah void * are totally evil. I mean, let's not mention that the standard libraries use them all the time.

Using void* internally, behind a robust, well-tested, and safe interface, is completely different from using void* all over your public interfaces. There are ways you could accomplish what you are doing much more neatly and powerfully than with void*s all over the place.


Anyways... not trying to be too harsh on you here or anything, but you did ask for criticism [wink]

I don't want to start an opinionated argument, but given we've(Tripwire Interactive) just hired a graphics programmer, I feel I must chime in to not steer this guy in the wrong direction.

As a graphics programmer, it is absolutely essential to be able to write the fastest code possible. When we're looking at a graphics programmer candidate, we are generally opposed to them using STL strings and vectors in their work, but other container constructs, such stacks, queues, deques are acceptable. The reasoning is simple, a graphics programmer *must* be able to handle the data themselves, as efficiently as possible, and automatic-allocation constructs such as strings and vectors are simply inefficient(and we would, in fact, call a graphics programmer that uses either of these constructs lazy). Also, when an engine is intended to be used by clients outside of the office, it's also imperative to write code that can easily be debugged, which the STL makes very difficult. In that light, we prefer that a graphics/engine level programmer simply doesn't even use the STL, but if they do happen to use the efficient container constructs, we simply inform them, upon hire, that they are not to use them in our source.

As StephenTC has agreed, there are a number of problems with his portfolio, but I do not believe using a const char* is one of them.

Quote:Original post by Xienen
When we're looking at a graphics programmer candidate, we are generally opposed to them using STL strings and vectors in their work, but other container constructs, such stacks, queues, deques are acceptable.

Dequeue automatically allocates in a manner similar to string and vector. Additionally, stack and queue are simple adapters for dequeue, meaning they share the same allocation characteristics. I have no idea what you thought you were saying in your statement, but it's apparent that you posses no idea of that which you speak.

Quote:Original post by Xienen
I don't want to start an opinionated argument, but given we've(Tripwire Interactive) just hired a graphics programmer, I feel I must chime in to not steer this guy in the wrong direction.

As a graphics programmer, it is absolutely essential to be able to write the fastest code possible. When we're looking at a graphics programmer candidate, we are generally opposed to them using STL strings and vectors in their work, but other container constructs, such stacks, queues, deques are acceptable. The reasoning is simple, a graphics programmer *must* be able to handle the data themselves, as efficiently as possible, and automatic-allocation constructs such as strings and vectors are simply inefficient(and we would, in fact, call a graphics programmer that uses either of these constructs lazy). Also, when an engine is intended to be used by clients outside of the office, it's also imperative to write code that can easily be debugged, which the STL makes very difficult. In that light, we prefer that a graphics/engine level programmer simply doesn't even use the STL, but if they do happen to use the efficient container constructs, we simply inform them, upon hire, that they are not to use them in our source.

As StephenTC has agreed, there are a number of problems with his portfolio, but I do not believe using a const char* is one of them.

Knowing the performance tendencies of the STL containers is paramount, and blanket statements such as "automatic-allocation constructs such as strings and vectors are simply inefficient" are bad.

Each container is efficient for the purposes it was designed for; string has a slight overhead but this is a tradeoff you may wish to make for greater security and convenience (nonetheless, its good for candidates to know how to deal with old school char *) while vectors simply wrap an old school array and have a trivial memory overhead, well worth the extra convenience, *if you are not concerned about performance in that area*.

Decisions about how to store memory and access it is always about tradeoffs; sometimes, a particular piece of code is simply not performance critical, (dont go all draconian on me and claim that all code is performance critical). Perhaps it runs once at game startup; Perhaps the difference in speed is immeasurably small.

For the sake of saving 1 millisecond of startup time you have just released a buggy game that leaks memory like a sieve. Congratulations; thats 20% less favorable critical reception.

Finish the sentence: premature optimisation __ ___ ____ __ __ ____.