Deck of cards as an interview exercise

I was asked to write a test code for an exercise for a job I applied for.

I was rejected, and I didn't know why. And I want to ask your opinions about my code.

The exercise is like that:


Write a simple class in the language of your choice to represent a
deck of cards with operations to shuffle the deck and to deal one
card. While not a requirement today, a likely future enhancement is
the need to deal all the cards in a deck. While not strictly
required, we value usage instructions, nicely-modeled data, automated
tests, and thoughtful consideration of architectural decisions and
simplicity-vs-completeness trade-offs.

I chose to write the code in TypeScript, because the position is front-end developer.

So, here is my code, and you tell me what I did wrong:

in the first file : card.ts

/* *** in the file Card.ts ***** */
    // Picture cards (Jack, Queen, King and Ace) will have matching numeric rank
    enum PictureCardType {Jack = 11, Queen = 12, King = 13, Ace = 1};

    enum SuiteType { Hearts , Diamonds, Spades, Clubs};

    class Card {
        private _rank: number;
        private _type: SuiteType;
        private _isPicture = function() : boolean {
           return this._rank > 10 || this._rank === 1;
        }

        constructor(rank: number, type: SuiteType) {
            if (rank > 0 && rank < 14)
                this._rank = rank;
            else
                throw new RangeError("card rank outside the range");
            this._type = type;
        }

        get rank() : number {
            return this._rank;
        }

        get suiteType() : SuiteType {
            return this._type;
        }
        // convert to JS object that has the following format: {suite: 'Dimaonds', rank : 'Ace'}
        toJSObject(): any {
            return {suite: SuiteType[this._type], rank: this._isPicture()  ?  PictureCardType[this._rank] : this._rank.toString() };
        }

    }

    export {Card , PictureCardType, SuiteType};

No one but the folks who reviewed your code for the company can say with certainty why you were rejected.

It also depends on the position you're applying for. If you're applying for an entry-level role or a guru role, the expectations are drastically different.

As a hiring manager, a couple things jumped out at me (from most to least relevant to me, a potential hiring manager):

-
You didn't follow directions.


While not a requirement today, a likely future enhancement is the need to deal all the cards in a deck.

You went rogue and spent time building a thing you were explicitly told was not a requirement.

This one is controversial. Some hiring managers will think, "Hey, he went above and beyond." Others will think, "I just asked you to build and shuffle a deck. Is this guy going to constantly try and adjust the requirements on the fly?"

I see both sides, but would err on the side of follow the directions you're given. Possibly include some comments describing how you would deal the cards and mentioning that you think it would be fairly trivial to add, so you would have talked to your lead/manager about adding it from the get go. Or implement it with a comment that you would have asked before going outside the spec, but assume the manager would have given their ok, so you built it.

-
Your shuffle test doesn't actually confirm that the deck was shuffled, just that it still exists at the same size. I want to see that the deck isn't in the same order, and that when I shuffle the original deck again, I get a new, third order. I want the UT to make sure the shuffle is at least pseudo-random.

-
The very first thing I noticed was your second line of code in Cards.ts. Cards do not have suites, they have suits. Some hiring managers might have immediately stopped looking. Misspelling variables is a maintenance nightmare.

-
Some of your comments have typos, are missing words, or use the wrong word ("help us chose" should be "help us choose")

Another nitpick, however, this is a job interview. This is your best foot forward. If you're moving to fast and missing details during the interview stage, why wouldn't you make more mistakes once we hire you?

-
The first time you use .slice(0), you have no comment, but the second time, you do. I suspect you wrote the second instance first along with the comment, but the first instance should have had the comment (if not each instance).

I liked that your CardDeckFactory was extensible to not just a hard coded standard 52 card deck, but could work with Pincochle, Eucher, or other non-standard decks.

I liked that you had the capacity for different shuffle algorithms.

Your code was generally clean, clear and easy to follow.

I like a number of your comments, particularly the longer, more descriptive ones. As someone unfamiliar with TypeScript, I appreciated the comment in the Unit Test about mocking. I also liked your comment about using a Generator. I hadn't seen that before, so it indicates to me that you have a pretty decent depth of understanding.

Others may have more technical reviews.

For what it's worth, if HR handed me the question and answer, I would give my thumbs up for a follow-on interview. In that follow-on, I would ask some questions around issue #1 to better understand why you went off spec.

Edited to add:

Every interviewer is going to key off of different things. Joseph The Dreamer's answer is likely correct in that you went too complex (though I think his answer likely goes too simple to the point of, again, disregarding the directions).

A good thing to keep in mind, especially if this is prior to a phone interview, the person reviewing this thing probably reviews dozens of them daily. Keeping it short, sweet and to the point is probably a good thing. There may be folks who see a couple hundred lines of code and would immediately reject you.

StackExchange Code Review Q#160427, answer score: 39