What is the "right" amount of time/effort to put into a "job-application" project like this?
These are not the speakers exact words but nearly..
... unless the role and the job post clearly state a particular framework as pre-requisite,
resorting to frameworks in an attempt to impress interviewers when applying to a senior role
is the worse thing you could do.
i.e. just because you know your way around Photoshop that doesn't make you a designer,
it makes you a Photoshop operator.
The same logic applies to coders who are too focus around frameworks.
They are not necessarily good programmers.
- The choice of using a Laravel back end was an informed decision to emulate as close as possible what would be my workflow on a real life app. Beside as opposed to front-end framework it has almost no cost attached and makes the back-end more maintainable and agile - Laravel is an amazingly clean framework. Finally the assessment of the basic PHP knowledge such as using PDO to connect MYSQL had already be made on paper test so there was no point repeating this there.
- The JS is certainly not perfect, game.js could use some refactoring, but my intent was not to make a production ready app but to show that I know how to write proper and maintainable vanilla JS. Given more time I would have addressed those details (which BTW are not the real issue in that code). Also you'll notice that I chose NOT to use a framework there as it was not needed. Again this was an informed decision.
- Sudo: I was worried that whom ever would try to install the app bumped into permission issues so I tried to make easier to install. I should have made a note of that. So point taken here. As for composer, I wouldn't want to work with any company that use PHP everyday and don't know what composer is.
You're right about your criticism of course, his memory game implementation is crap in many, many ways. But in the grand scheme of things, it's details. Most developers in the real world wouldn't be able at all to put it together in javascript in only two days (i've worked with people who wouldn't have been able to do it in two months). Hell, a large minority of developers can't solve the FizzBuzz test so disqualifying this guy for using to many dependencies and requiring sudo is silly.
I think the best way to get feedback on a rejected code sample is to give your code to an objective third party and ask their opinion, like OP did here.
If you want to respond, respond to those items, not excuses <- denotes junior-level mentality.
Why the dummy account ?
Remember, when you're looking for a job, you're not looking to create some build and forget application. You're going to be working as part of a team. Other people need to be able to maintain your code. For a problem that should be able to fit onto a single page, you don't want to have to browse through a 7 megabyte git repo with nearly 10,000 files in it.
An analogy might be if someone's trying to hire you as a carpenter. They ask you to build a cabinet to demonstrate your skill. You go and find a prefab house frame, an entire prefab plumbing system, and so on, and build the skeleton of a house. Within that, you build a reasonably functional cabinet, though it's kind of hard to judge the cabinet among the entire rest of the incomplete house. How do you think that's going to compare to someone who just went and build a cabinet, and spent the time getting it built really nicely, paid attention to the detail work, made sure the door hung true and opened and closed without squeaks?
Here are a few things that I note at a cursory glance; and notice that I haven't even gotten to the logic of the code, because all of these things are red flags that come up before I am even able to read it:
1. Checking in dependencies to Git. That's not how it you do it; dependencies should be handled by a separate package manager, in which you depend on the appropriate versions of packages. You should never check anything other than your own source code into Git.
2. Your commit messages. Commit messages in Git should follow the format of an email describing a patch. The first line should summarize what the commit does; the rest of the commit should contain a description of why you are doing it. See the following for an explanation of what good commit messages should look like. First from the Git source itself:
http://git.kernel.org/cgit/git/git.git/tree/Documentation/Su...
And then a much more in depth guide:
https://wiki.openstack.org/wiki/GitCommitMessages#Informatio...
3. Inappropriate use of frameworks. You included a lot of dependencies that are unnecessary for the problem at hand. Dependencies have real costs; every extra dependency is something that could break, could have a security bug, could be a pain to upgrade in the future. Now, that doesn't mean to never add dependencies; but do so with a certain amount of care.
4. Simple spelling and style errors. Tabs vs. spaces. Inconsistent spacing: "var _createCell = function( color, id ) {" and "if ( el.className.indexOf('turned-over') > -1)" (note: decide whether there are spaces between parens and their contents, and stick to that choice). You spelled "RESTful" as "restFul". Things like this are seen by developers as canaries in the coal mine; if you are sloppy about the very basic stuff like tabs vs. spaces, are you really going to be mindful of other important things? Consistency of style is important for keeping code readable, if you can't even keep consistent in a simple code interview problem, why should anyone think that you'll be able to do it on a day to day basis when under real deadline pressure?
Here's my exercise for you. If you want to get better, and be able to pass future job interviews, condense this down into as small a codebase as you can, with as few dependencies as possible. I think that you should be able to do this in less than 100 lines of your own code, and only the most basic of dependencies like jQuery and some simple microframework on the server side.
Give that a shot, and then take a look at both for comparison. Which would you rather maintain? Which will be easier to make changes to? Which is easier to code review?
@OP: You didn't commit the composer.lock. That means that your code might run locally but breaks on another machine because there might be a new version of laravel 4.2
Making things too abstract, too early, is usually a bad sign. In most cases, the most most specific and straight forward solution to a problem is the best one.
There's definitely cases where you're simplifying things by making them more abstract and reusable (but making something reusable before you need to reuse it is a waste of time) -- another good case is where you're using the principle of separation of concerns.
I think when I started developing I made everything very abstract, b/c all the books you read would also tell you that was a good thing.
It's not. Until you need to, at least.
Abstraction can be a great tool, but it comes with the cost of added indirection.
You want to aim for the sweet spot in the middle somewhere.
tl;dr: I agree with the assessment that this doesn't look like the kind of code I'd expect of a senior developer.
The biggest issue from my POV is that you pull in a _lot_ of heavy-weight frameworks and implicit dependencies for what could be a really simple app. To actually examine the application logic of your submission, I have to flip through dozens of auto-generated PHP files, Javascript module wrappers, and empty directories.
Sometimes, calling yourself a "senior" engineer means doing less, not more. The layers of indirection, framework code, and build toolchain applied here make it harder to follow what your code is actually doing. Coupled with a lack of unit tests, it seems impossible to verify in any short amount of time that it actually works as promised.
Even with all that MVC goodness, the separation of concerns on the client isn't great. There's a mix of display logic and app state visible in lots of places. (IMHO game.js is the worst offender here, as the entire game state and DOM rendering are mixed pretty indiscriminately.)
Finally, asking a reviewer to run a bunch of code from an untrusted repo via 'sudo' is pretty much a giant red flag for me. A simple JS+PHP webapp shouldn't require me to do anything as root on my machine. (I'm also not familiar with Composer, but my brief skimming of the homepage, it seems like the whole point is to not require system-wide installation of dependencies, so the sudo bit is especially weird.)
It looks functional, and if you were interviewing specifically at a shop that used these frameworks and tools, it might not be so bad. If asked to review this without that context, though, I would give similar feedback to what you received.