BlackJack in Angular

I wrote the following code in Angular as to accomplish a technical test for a UK job. My impression was that it is very neat and well object oriented. To my big surprise, they rejected it. Could someone find any problematic points as to ameliorate my coding performance?



`angular.module('myApp', []).
controller('myController', ['$scope', function ($scope) {
var Deck=['C10','D10','H10','S10','C2','D2','H2','S2','C3','D3','H3','S3','C4','D4','H4','S4','C5','D5','H5','S5','C6','D6','H6','S6',
'C7','D7','H7','S7','C8','D8','H8','S8','C9','D9','H9','S9','CA','DA','HA','SA','CJ','DJ','HJ','SJ','CK','DK','HK','SK','CQ','DQ','HQ','SQ'];
$scope.finalMessage='';

function blackJackGame(){
this.Deck=shuffle(Deck);
this.dealer=new Player('dealer');
this.player=new Player('player');
}

function Player(name){
this.name=name;
this.score=[0,0];
this.aces=0;
this.cards=[];
}

Player.prototype.Hit=function(value,arr){
self=this;
this.cards.push(arr[value]);
if (arr[value].slice(1)==='J' || arr[value].slice(1)==='K' || arr[value].slice(1)==='Q') {
this.score[0]+=10;
this.score[1]+=10;
calculate(self);
}
else if (arr[value].slice(1)==='A') {
this.aces++;
if (this.aces>1) {
this.score[0]++;
this.score[1]++;
}
else {
this.score[0]++;
this.score[1]+=11;
}
calculate(self);
}
else {
this.score[0]+=parseInt(arr[value].slice(1));
this.score[1]+=parseInt(arr[value].slice(1));+
calculate(self);
}
}; //end of Player.prototype.Hit

function calculate(self){
if (self.name==='player') {
if (self.score[0]===self.score[1]) {
$scope.playerScore=self.score[0];
$scope.playerCards=self.cards;
}
else {
if (self.score[1]21){
$scope.finalMessage="PLAYER LOSES";
$scope.enableButtons=false;
}
else
counter++;
};

Yes, I agree that the code style is not great and there are several design issues here as well. For example, why does the main game object have a shuffle() function? The Deck should know how to shuffle itself.

Some of your function names are hard to understand. calculate() is kind of meaningless. And when I see value and arr as variable names, I cringe a bit. Names of things are very important. Your functions are generally too long, too, which is usually a sign that they are trying to do too much or that you have other design issues. Try to make them 5-15 lines, though that is only a rule of thumb.

I would look into design patterns but also at refactoring in general. A long conditionals like arr[value].slice(1)==='J' || arr[value].slice(1)==='K' || arr[value].slice(1)==='Q' does not read well and could be its own one-line function instead, like isSuitCard(). Or, better yet, what if it was card.isSuit()? That would look great, but it would require you to turn this Deck array plus chosen value into a proper Card object. Maybe that's not such a bad idea?

A good goal is to have code that's pleasant for others to pick up and read, especially those who have never seen it before.

Finally, you haven't written any tests and your code isn't easily testable. Another red flag there. I would suggest looking into test-driven development because the discipline of that approach will make you a better coder faster.

StackExchange Code Review Q#127736, answer score: 4