HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Diamond kata in JavaScript

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascriptdiamondkata

Problem

I'm just starting to learn JavaScript. I implemented the Diamond kata, and I would appreciate if anybody could give me some feedback. I'm particularly concerned about learning the most 'idiomatic' way to do things in JavaScript, style conventions, etc.

The diamond kata consists of the following problem statement:

Given a letter, print a diamond starting with ‘A’ with the supplied letter 
at the widest point.

For example: print-diamond ‘C’ prints

  A
 B B
C   C
 B B
  A


The production code (test code in the Gist):

// Diamond Kata with a TDD approach. First attempt
'use strict';

module.exports.Diamond = Diamond;

function Letter(char, spaces) {
this._char = char;
this._spaces = spaces;
}
Letter.prototype.print = function print() {
return this._char + (
(this._spaces > 0)
? (Array(this._spaces + 1).join(' ') + this._char + '\n')
: '\n'
);
}

// Singleton list with all letters
var LETTERS = new function () {
// Init all upper-case letters
for (let char=65; char = 0) {
callback(this[chars[pos]], pos, last);
if (pos === last) {
// Reached final character, return to 'A'
direction = -1;
}
pos += 1 * direction;
}
}

function Diamond (letter) {
if (typeof(letter) !== 'string' && !(letter instanceof String)) {
throw new Error('Letter must be an string');
}
this._letter = letter;
};
Diamond.prototype.process = function process() {
var prefix = function (max, current) {
return Array(1 + max - current).join(' ');
}
var result = '';
LETTERS.iterateLetters(this._letter, function (letter, current, last) {
result += prefix(last, current);
result += letter.print();
});
return result;
}


This code is meant to execute at a "Node --harmony" environment, although except from the module exportation, this is just plain JavaScript.

A working demo is available here.

This is written using a TDD approach. The test code, as well as this same snippet, can be found in the Gist.

Solution

I think you are overthinking it. Or maybe this is just not the right problem for TDD? This seems to be a typical interview / Project Euler (kind of) question. Something that doesn't require more testing than running it several times in console.

Why do I say this?--Tests are typically needed when the borderline conditions for the input are not trivially found and the algorithm being tested may take on multiple ways, not easily seen from its definition. In this case we have a very clearly stated borderline condition for inputs (our inputs are characters A-Z) and the algorithm may only take on two paths (a base-case, when 'A' is given) and the "other" cases (given any letter other than 'A'). The algorithm isn't defined for any other input.

Below is a simple version of this problem:

function diamondKata(character) {
    var a = 'A'.charCodeAt(), frame, line,
        width = character.toUpperCase().charCodeAt() - a,
        i = 0, lines = [];

    function blanks(n) { return new Array(n).join(' '); }

    function makeLine() {
        return blanks(width + 1 - i) + frame +
            blanks(i * 2) + frame;
    }
    while (i++ < width) {
        frame = String.fromCharCode(a + i);
        lines.push(makeLine());
    }
    if (lines.length) {
        lines.unshift(blanks(width + 1) + 'A');
        lines = lines.concat(
            lines.slice(0, lines.length - 1).reverse());
    } else {
        lines.push('A');
    }
    return lines.join('\n');
}


Some points about your code: while operations with strings are generally very fast in JavaScript (highly optimized C code), it is still better to create long strings by first pushing all chunks into an array, and then to join it all in one go. The reason for this is that creating strings by consequent concatenation will make a lot of intermediate memory allocations, which would be avoided if the string is created at once.

More on your code: type checking is frowned upon, unless its some library code, where the algorithm may take on multiple paths depending on what kind of object it works with. When you check types only to throw a type error, you are working against the benefits you get from dynamically type-checked language: with dynamically type-checked languages you get these type errors for free (the environment already does that for you).

Compare this to C-like languages, where after dereferncing a pointer and making wrong assumption about the type of the object pointed by the derefernced pointer will spell disaster for the program, unless checked.

Hardcoding the codepoint assigned to characters would be a bad thing to do in any language, unless it's an optimization justified by a profiling session.

Code Snippets

function diamondKata(character) {
    var a = 'A'.charCodeAt(), frame, line,
        width = character.toUpperCase().charCodeAt() - a,
        i = 0, lines = [];

    function blanks(n) { return new Array(n).join(' '); }

    function makeLine() {
        return blanks(width + 1 - i) + frame +
            blanks(i * 2) + frame;
    }
    while (i++ < width) {
        frame = String.fromCharCode(a + i);
        lines.push(makeLine());
    }
    if (lines.length) {
        lines.unshift(blanks(width + 1) + 'A');
        lines = lines.concat(
            lines.slice(0, lines.length - 1).reverse());
    } else {
        lines.push('A');
    }
    return lines.join('\n');
}

Context

StackExchange Code Review Q#79333, answer score: 2

Revisions (0)

No revisions yet.