4 Reasons To Drop MVVM

2016-07-27

The MVVM design pattern has been around for quite a while now. It has a lot of strengths when done correctly.

But, I believe the time has come to recognize that MVVM has a lot of shortcomings that point to its demise. Since I primarily develop web applications, I will keep this discussion centered on the use of MVVM in web applications. The use of MVVM for desktop may or may not have these same issues.

I realize that for some of you, the very suggestion of dropping MVVM will invoke a negative emotional response. Some very smart people have quit their job at the suggestion that MVVM and its close cousin two-way data-binding, be abandoned in favor of another way. But just for a few minutes, I would like for you to stop treating programming as a religion and consider the possibility that there may be a better way.

History of MVVM

MVVM was originally created by John Gossman to support the XAML syntax used to create Windows™ desktop applications and Silver Light applications. Its main advantage has always been that it provides an easy way to decouple the View code from any business logic that might need to run. Because of this decoupling, our applications become much easier to unit test.

The next major implementation of MVVM that I can remember is Knockout. It is the knockout framework that introduced me to MVVM and I have to say it is also the only one I feel like actually got it right. By that I mean that it actually did what it was advertised to do. Maybe that’s because of all the implementations I’ve used, Knockout is the only one that ONLY implemented MVVM rather than making it part of a larger framework.

Definition of MVVM

I’ve written about MVVM before where I’ve explained more completely what MVVM is. But just so we have a working definition of what I mean when I talk about MVVM, let’s define it this way.

MVVM is a design pattern that uses two-way data-binding to get data in and out of the presentation layer, referred to as the View, without the programmer needing to do any more than specifying that this should happen in the view. MVVM is also able to have data elements and functions in the “ViewModel” track changes so that anything that is dependent on other data is automatically recalculated without the programmer having to write a lot of code to make this happen. This creates a model that is able to respond to events in the view, but is primarily data centric rather than event centric has we have often reasoned about our applications in the past.

It sounds great. And when it works it is. But that’s the main problem, it hardly ever works well.

MVVM Done Right is Slow

If you’ve had any experience or paid any attention to the implementation of MVVM using JavaScript, you will realize that the number one problem with MVVM is that it is a memory hog and performs poorly for all but the most trivial of applications. In fact, for all of the popularity of Angular JS, the biggest complaint has been around the implementation of the data-binding. In a large application, you might need to loop through the data multiple times to make sure it has all recalculated correctly. If you just use the framework and let the framework deal with your sloppy code, this can make the system incredibly slow. If you actually pay attention to what you are doing, it takes longer to implement than if you had chosen some other design pattern.

But doing that means we have not moved on to…

MVVM is Hard to Implement

Recognizing that looping through the data until it stabilizes may not be a good idea, the framework designers have developed rules such as, “We’ll only run the digest cycle once.” and “We’ll only run it when some user interaction has occurred.” Well, OK. That sounds good. At least now it will be obvious that I have a problem. But this is where the trouble begins. If I can’t rely on my data, and ultimately my view, responding to changes in my data correctly, I am left with having to only partially implementing MVVM so that I can work around these limitations and using other means to make sure my view is updated correctly.

This is to say nothing of many frameworks just not working as you would expect them to.

MVVM is Hard to Reason About

Again, in all but the most trivial of applications, and because of the optimizations that various frameworks have tried to implement, MVVM becomes difficult to implement. As I’ve tried to explain MVVM to others and even as I’ve tried to implement it myself, I’ve found that the simple act of keeping the view stuff in the view layer and the data stuff in the data layer and making sure it all updates appropriately has me, at times tearing my hair out. Many times this is caused by incomplete implementations. But if being hard to implement means it is hard to reason about, maybe we shouldn’t be using it to begin with.

MVVM is Overkill

But it does work sometimes. In really simple CRUD applications, it works great. None of the problems I’ve mentioned. And this is the great seduction of MVVM. You try it on some small application and you get excited. Like a gateway drug, it lures you in. And when you finally go to implement it on some larger application, you find out that it really doesn’t scale all that well. And on that small app you tried it on, couldn’t you have done that just as easily using another design pattern?

Where to Go from Here

As I was reviewing these arguments with a co-worker this week, he asked, “Are you saying we shouldn’t be using MVVM?” And my answer might surprise you.

I said, “Given the two models we have to work within the framework we are currently using, MVVM is the best choice.” However, what we might want to consider is moving to another framework that provides a better design pattern. There are several “One-way” design patterns that intrigue me. The first is the basic Flux pattern that React tends to use. Done correctly, this uses events to achieve the decoupling we all should be striving for. At its core, it is basic MVC.

The second one, which is very flux like, is RxJS. I’m still wrapping my head around how I to use it in an application and honestly don’t know enough about it at this point to say any more than that it looks interesting.

And even if we decided to move away from MVVM, I think using two way data-binding between the view and the ViewModel is good. I just think the ViewModel shouldn’t try to re-compute the values as part of what it does.

Leave that to the developer to control. The problem is, trying to get existing systems that implement two-way data-binding to only work at that level would not work correctly.

Other places talking about MVVM

The Advantages and Disadvantages of MVVM (by John Gossman himself!)

Test Driven Development Kata - Roman to Arabic in JavaScript

2016-07-20

TDD

Coding Katas are a way of developing your skills as a programmer. I thought it might be informative to tackle one of the classics as a blog post. Depending on how this works, I may or may not do another one quite so publicly. The rules I’m going to try to adhere by.

I will document what I am doing as I go.
This is not a pre-coded blog post. You’ll get to “see” me code as I go.
I will write all tests first.
I will only write enough code to make the current tests succeed.

Test Driven Kata - Roman to Arabic

Today’s problem:

This is a pretty classic coding problem that shows up in interviews, home-work assignments, and code katas.

As an interview problem, I find it lacking because it typically does not represent the kind of work you will be doing other than proving that you can solve problems in your chosen language. It is also a standard coding problem, meaning the person interviewing you is using the most obvious problem to see if you can code. Much like asking “What is your greatest strength?” and “What is your greatest weakness?” Finally, it takes longer to complete than I believe interview coding problems should. This is not to say, I don’t jump through these hoops myself.

As a kata exercise, it is good because there are several ways you might solve the problem. And for our purposes, it also demonstrates what Test Driven Development might look like using JavaScript.

So, what is the problem? Write a function that converts Roman numbers into Arabic numbers and throws an error if the Roman number is in an invalid form.

That sounds pretty easy. But the first question we need to ask is, “What, exactly are the rules for converting Roman numbers into Arabic numbers?”

The values for Roman numbers are as follows:

Roman	Arabic
I	1
V	5
X	10
L	50
C	100
D	500
M	1000

Repeating a number up to three times adds that value three times.
A Roman ‘digit’ can’t repeat more than three times. Instead the previous 1, 10, or 100 equivalent value is used to subtract from the next ‘digit’. That is,

Roman	Arabic
IV	4
IX	9
XL	40
XC	90
CD	400
CM	900

With the exception of the subtraction rule above, all values must decrease in scale from left to right and are added together.

Test 1

Since we will be using JavaScript, the testing framework we will be using is Jasmine.

Our first test will simply test that when we pass in “I” we get back 1.

describe('tests/roman-to-arabic/RomanToArabic.spec.js',function(){
    var returnValue;
    describe('When I is passed in',function(){
        beforeEach(function(){
            returnValue = romanToArabic("I");
        });
        it('should return 1',function(){
            expect(returnValue).toBe(1);
        });
    })
});

And the code that passes this test is:

1
2
3

function romanToArabic(romanNumber){
    return 1;
}

You might think, what’s the point? Why just return 1 when you know you are going to have to do more? Well, when you are doing TDD, you have to work off of what you are testing for now, not what you might test for later. So, we return 1.

Test 2

From here, we can go a couple of different directions. How about testing for rule 3 next. How would we do that? To start with, we might just make sure that if we pass in IV, we get back 4. Now our code is getting a bit more complicated. But sticking with our TDD principles, we’ll just test for IV. Another happy path test.

describe('tests/roman-to-arabic/RomanToArabic.spec.js',function(){
    var returnValue;
    describe('When I is passed in',function(){
        beforeEach(function(){
            returnValue = romanToArabic("I");
        });
        it('should return 1',function(){
            expect(returnValue).toBe(1);
        });
    })
    describe('When IV is passed in', function(){
        beforeEach(function(){
            returnValue = romanToArabic("IV");
        });
        it('should return 4',function(){
            expect(returnValue).toBe(4);
        });
    })
});

And the code that implements it:

function romanToArabic(romanNumber){
    switch(romanNumber){
        case 'I':
            return 1;
        case 'IV':
            return 4;
    }
}

Test 3

OK. What happens if IV shows up twice? That should be a failure. IV should never show up more than once. In fact, none of the codes that show up in rule three should show up more than once. Let’s make sure they don’t.

The tests for this will be pretty simple, and to save time, we will code for all of them at once. In fact, we are going to eventually need the conversion tables above, so let’s go ahead and put them in now.

Here is what our test file looks like now. Notice that we’ve put several tests in at once.

Notice that I didn’t have to write multiple tests to do this. I just created one test and iterated over it.

describe('tests/roman-to-arabic/RomanToArabic.spec.js',function(){
    var returnValue;
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    describe('When I is passed in',function(){
        beforeEach(function(){
            returnValue = romanToArabic('I');
        });
        it('should return 1',function(){
            expect(returnValue).toBe(1);
        });
    });
    describe('When IV is passed in', function(){
        beforeEach(function(){
            returnValue = romanToArabic('IV');
        });
        it('should return 4',function(){
            expect(returnValue).toBe(4);
        });
    });
    for(var prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
});

And the solution also iterates.

function romanToArabic(romanNumber){
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    var baseTable = {
        I: 1,
        V: 5,
        X: 10,
        L: 50,
        C: 100,
        D: 500,
        M: 1000
    };

    // This block is probably inefficient, but it is
    // easy to reason about.
    for(var prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
           continue;
        }
        var rx = new RegExp(prop,'g');
        if((romanNumber.match(rx) || []).length > 1){
            throw 'Poorly formed Roman number!';
        }

    }

    switch(romanNumber){
        case 'I':
            return 1;
        case 'IV':
            return 4;
    }
}

Test 4

Just to make sure we aren’t counting how many times in a row this all shows up, let’s add a valid Roman Number in between.

describe('tests/roman-to-arabic/RomanToArabic.spec.js',function(){
    var returnValue;
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    describe('When I is passed in',function(){
        beforeEach(function(){
            returnValue = romanToArabic('I');
        });
        it('should return 1',function(){
            expect(returnValue).toBe(1);
        });
    });
    describe('When IV is passed in', function(){
        beforeEach(function(){
            returnValue = romanToArabic('IV');
        });
        it('should return 4',function(){
            expect(returnValue).toBe(4);
        });
    });
    var prop;
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + 'I' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy + 'I' + propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
});

Test 5

Next, we need to make sure that none of the items in our base table show up more than 3 times. We’ll write a similar test to what we’ve already written with a twist. If any of those numerals show up more than 4 times, we know we have a problem either because they are out of order or because they show up all in a row. So, we only need to check the count. If there are four of any of them, we have a problem.

describe('tests/roman-to-arabic/RomanToArabic.spec.js',function(){
    var returnValue;
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    var baseTable = {
        I: 1,
        V: 5,
        X: 10,
        L: 50,
        C: 100,
        D: 500,
        M: 1000
    };
    describe('When I is passed in',function(){
        beforeEach(function(){
            returnValue = romanToArabic('I');
        });
        it('should return 1',function(){
            expect(returnValue).toBe(1);
        });
    });
    describe('When IV is passed in', function(){
        beforeEach(function(){
            returnValue = romanToArabic('IV');
        });
        it('should return 4',function(){
            expect(returnValue).toBe(4);
        });
    });
    var prop;
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + 'I' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy + 'I' + propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in baseTable){
        if(!baseTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy+propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
});

And the code to implement it.function romanToArabic(romanNumber){
var minusOneTable = {
IV: 4,
IX: 9,
XL: 40,
XC: 90,
CD: 400,
CM: 900
};
var baseTable = {
I: 1,
V: 5,
X: 10,
L: 50,
C: 100,
D: 500,
M: 1000
};

// This block is probably inefficient, but it is
// easy to reason about.
for(var prop in minusOneTable){
    if(!minusOneTable.hasOwnProperty(prop)){
       continue;
    }
    var rx = new RegExp(prop,'g');
    if((romanNumber.match(rx) || []).length > 1){
        throw 'Poorly formed Roman number!';
    }

}
for(var prop in baseTable){
    if(!baseTable.hasOwnProperty(prop)){
        continue;
    }
    var rx = new RegExp(prop,'g');
    if((romanNumber.match(rx) || []).length > 3){
        throw 'Poorly formed Roman number!';
    }

}

switch(romanNumber){
    case 'I':
        return 1;
    case 'IV':
        return 4;
}

}

Test 6

So, before we move on to actually computing the value of the Roman number, we should ask ourselves if there are any other ways a Roman number could be passed in incorrectly.

The next one that occurs to me is this. If you have a number that contains a ‘digit’ from the minusOneTable, the character that is used to subtract should never follow the digit. For example, if “IV” shows up, there should not be an “I” immediately after it. That is, we shouldn’t see “IVI” anywhere in our string. So let’s add that to our tests.

describe('tests/roman-to-arabic/RomanToArabic.spec.js',function(){
    var returnValue;
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    var baseTable = {
        I: 1,
        V: 5,
        X: 10,
        L: 50,
        C: 100,
        D: 500,
        M: 1000
    };
    describe('When I is passed in',function(){
        beforeEach(function(){
            returnValue = romanToArabic('I');
        });
        it('should return 1',function(){
            expect(returnValue).toBe(1);
        });
    });
    describe('When IV is passed in', function(){
        beforeEach(function(){
            returnValue = romanToArabic('IV');
        });
        it('should return 4',function(){
            expect(returnValue).toBe(4);
        });
    });
    var prop;
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + 'I' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy + 'I' + propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in baseTable){
        if(!baseTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy+propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        var newProp = prop + prop.substr(0,1);
        describe('When ' + newProp + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,newProp));
    }
});

And the code to implement it:

function romanToArabic(romanNumber){
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    var baseTable = {
        I: 1,
        V: 5,
        X: 10,
        L: 50,
        C: 100,
        D: 500,
        M: 1000
    };

    // This block is probably inefficient, but it is
    // easy to reason about.
    var prop;
    var rx;
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
           continue;
        }
        rx = new RegExp(prop,'g');
        if((romanNumber.match(rx) || []).length > 1){
            throw 'Poorly formed Roman number!';
        }
        rx = new RegExp(prop + prop.substr(0,1),'g');
        if((romanNumber.match(rx) || []).length > 0){
            throw 'Poorly formed Roman number';
        }

    }
    for(prop in baseTable){
        if(!baseTable.hasOwnProperty(prop)){
            continue;
        }
        rx = new RegExp(prop,'g');
        if((romanNumber.match(rx) || []).length > 3){
            throw 'Poorly formed Roman number!';
        }

    }

    switch(romanNumber){
        case 'I':
            return 1;
        case 'IV':
            return 4;
    }
}

Step 7

There is one more possible problem we could encounter. What if someone passes in a character that isn’t a valid Roman number character. We need to make sure that the only characters that show up are Roman number characters.

Since testing for all of the characters isn’t practical, we are just going to test for a few and assume that if this were a real business problem we’d go to the trouble of testing more. But, the basic test is going to look the same. Toss in bad characters in an otherwise valid string and make sure we throw an error.

describe('tests/roman-to-arabic/RomanToArabic.spec.js',function(){
    var returnValue;
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    var baseTable = {
        I: 1,
        V: 5,
        X: 10,
        L: 50,
        C: 100,
        D: 500,
        M: 1000
    };
    describe('When I is passed in',function(){
        beforeEach(function(){
            returnValue = romanToArabic('I');
        });
        it('should return 1',function(){
            expect(returnValue).toBe(1);
        });
    });
    describe('When IV is passed in', function(){
        beforeEach(function(){
            returnValue = romanToArabic('IV');
        });
        it('should return 4',function(){
            expect(returnValue).toBe(4);
        });
    });
    var prop;
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + 'I' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy + 'I' + propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in baseTable){
        if(!baseTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy+propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        var newProp = prop + prop.substr(0,1);
        describe('When ' + newProp + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,newProp));
    }
    // just test for one each of upper case, lower case, number and lower case valid character
    var invalidCharacters = {A:1,a:1,'2': 1, i: 1};
    for(prop in invalidCharacters){
        if(!invalidCharacters.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
});

And the solution, which is pretty simple:

function romanToArabic(romanNumber){
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    var baseTable = {
        I: 1,
        V: 5,
        X: 10,
        L: 50,
        C: 100,
        D: 500,
        M: 1000
    };

    // This block is probably inefficient, but it is
    // easy to reason about.
    var prop;
    var rx;
    // make sure minusOne only shows up once
    // and first character isn't also the last character. (IVI for example)
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
           continue;
        }
        rx = new RegExp(prop,'g');
        if((romanNumber.match(rx) || []).length > 1){
            throw 'Poorly formed Roman number!';
        }
        rx = new RegExp(prop + prop.substr(0,1),'g');
        if((romanNumber.match(rx) || []).length > 0){
            throw 'Poorly formed Roman number';
        }

    }

    var included = '';
    // make sure digits only show up 3 times

    for(prop in baseTable){
        if(!baseTable.hasOwnProperty(prop)){
            continue;
        }
        included += prop;
        rx = new RegExp(prop,'g');
        if((romanNumber.match(rx) || []).length > 3){
            throw 'Poorly formed Roman number!';
        }
    }

    // make sure only I, V, X, L, C and D are the only characters that show up
    rx = new RegExp('[^' + included + ']','g')
    if((romanNumber.match(rx) || []).length > 0){
        throw 'Poorly formed Roman number';
    }


    switch(romanNumber){
        case 'I':
            return 1;
        case 'IV':
            return 4;
    }
}

Step 8

OK. I think that gets all of the validations except for ensuring that the numbers show up in numerical order. To make this work, we are going to need to work through the array and assign each digit a value. The test we need to write is going to put values in the wrong order.

Our test will put the next value up after the value.

describe('tests/roman-to-arabic/RomanToArabic.spec.js',function(){
    var returnValue;
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    var baseTable = {
        I: 1,
        V: 5,
        X: 10,
        L: 50,
        C: 100,
        D: 500,
        M: 1000
    };

    describe('When I is passed in',function(){
        beforeEach(function(){
            returnValue = romanToArabic('I');
        });
        it('should return 1',function(){
            expect(returnValue).toBe(1);
        });
    });
    describe('When IV is passed in', function(){
        beforeEach(function(){
            returnValue = romanToArabic('IV');
        });
        it('should return 4',function(){
            expect(returnValue).toBe(4);
        });
    });
    var prop;
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + 'I' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy + 'I' + propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in baseTable){
        if(!baseTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy+propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        var newProp = prop + prop.substr(0,1);
        describe('When ' + newProp + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,newProp));
    }
    // just test for one each of upper case, lower case, number and lower case valid character
    var invalidCharacters = {A:1,a:1,'2': 1, i: 1};
    for(prop in invalidCharacters){
        if(!invalidCharacters.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    var badOrderPairs = {DM: 1, LC: 1, VX: 1, IXXL: 1, IXM: 1};
    for(prop in badOrderPairs){
        if(!badOrderPairs.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
});

And the solution will put them in order and add them along the way.

function romanToArabic(romanNumber){
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    var baseTable = {
        I: 1,
        V: 5,
        X: 10,
        L: 50,
        C: 100,
        D: 500,
        M: 1000
    };
    var minusOneSub = {
        IV: 'v',
        IX: 'x',
        XL: 'l',
        XC: 'c',
        CD: 'd',
        CM: 'm'
    };
    var compositeValueTable = {
        I: 1,
        v: 4,
        V: 5,
        x: 9,
        X: 10,
        l: 40,
        L: 50,
        c: 90,
        C: 100,
        d: 400,
        D: 500,
        m: 900,
        M: 1000
    };

    // This block is probably inefficient, but it is
    // easy to reason about.
    var prop;
    var rx;
    // make sure minusOne only shows up once
    // and first character isn't also the last character. (IVI for example)
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
           continue;
        }
        rx = new RegExp(prop,'g');
        if((romanNumber.match(rx) || []).length > 1){
            throw 'Poorly formed Roman number!';
        }
        rx = new RegExp(prop + prop.substr(0,1),'g');
        if((romanNumber.match(rx) || []).length > 0){
            throw 'Poorly formed Roman number';
        }
    }

    var included = '';
    // make sure digits only show up 3 times

    for(prop in baseTable){
        if(!baseTable.hasOwnProperty(prop)){
            continue;
        }
        included += prop;
        rx = new RegExp(prop,'g');
        if((romanNumber.match(rx) || []).length > 3){
            throw 'Poorly formed Roman number!';
        }
    }

    // make sure only I, V, X, L, C and D are the only characters that show up
    rx = new RegExp('[^' + included + ']','g');
    if((romanNumber.match(rx) || []).length > 0){
        throw 'Poorly formed Roman number';
    }

    // substitute the minusOnes with tokens we can use to compute value.
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        rx = new RegExp(prop,'g');
        romanNumber = romanNumber.replace(rx,minusOneSub[prop]);
    }

    var romanNumberArray = romanNumber.split('');
    var returnValue = 0;
    var lastValue = 0;
    var currentValue = 0;
    for(var numberIndex = 0;numberIndex < romanNumberArray.length;numberIndex++){
        currentValue = compositeValueTable[romanNumberArray[numberIndex]];
        returnValue += currentValue;
        if(numberIndex > 0 && currentValue > lastValue){
            throw 'Poorly formed Roman number';
        }
        lastValue = currentValue;
    }
    return returnValue;

    // switch(romanNumber){
    //     case 'I':
    //         return 1;
    //     case 'IV':
    //         return 4;
    // }
}

Step 9

And finally we add some test to verify that we get the right result.

describe('tests/roman-to-arabic/RomanToArabic.spec.js',function(){
    var returnValue;
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    var baseTable = {
        I: 1,
        V: 5,
        X: 10,
        L: 50,
        C: 100,
        D: 500,
        M: 1000
    };

    describe('When I is passed in',function(){
        beforeEach(function(){
            returnValue = romanToArabic('I');
        });
        it('should return 1',function(){
            expect(returnValue).toBe(1);
        });
    });
    describe('When IV is passed in', function(){
        beforeEach(function(){
            returnValue = romanToArabic('IV');
        });
        it('should return 4',function(){
            expect(returnValue).toBe(4);
        });
    });
    var prop;
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + 'I' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy + 'I' + propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in baseTable){
        if(!baseTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy+propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        var newProp = prop + prop.substr(0,1);
        describe('When ' + newProp + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,newProp));
    }
    // just test for one each of upper case, lower case, number and lower case valid character
    var invalidCharacters = {A:1,a:1,'2': 1, i: 1};
    for(prop in invalidCharacters){
        if(!invalidCharacters.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    var badOrderPairs = {DM: 1, LC: 1, VX: 1, IXXL: 1, IXM: 1};
    for(prop in badOrderPairs){
        if(!badOrderPairs.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    var romanNumbers = {
        I: 1,
        II: 2,
        III: 3,
        IV: 4,
        V: 5,
        VI: 6,
        VII: 7,
        VIII: 8,
        IX: 9,
        X: 10,
        XI: 11,
        XII: 12,
        XIII: 13,
        XIV: 14,
        XV: 15,
        XVI: 16,
        XVII: 17,
        XVIII: 18,
        XIX: 19,
        XX: 20,
        XXI: 21,
        XXII: 22,
        XXIII: 23,
        XXIV: 24,
        XXV: 25,
        XXVI: 26,
        XXVII: 27,
        XXVIII: 28,
        XXIX: 29,
        XXX: 30,
        XL: 40,
        XLIV: 44,
        XLV: 45,
        XLVI: 46,
        L: 50,
        LIV: 54,
        LV: 55,
        LVI: 56,
        LX: 60,
        XC: 90,
        C: 100,
        CI: 101,
        CXI: 111,
        CMXCIX: 999,
        MDCLXVI: 1666

    };
    for(prop in romanNumbers){
        if(!romanNumbers.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            beforeEach(function(){
                returnValue = romanToArabic(propCopy);
            });
            it('should return ' + romanNumbers[propCopy] ,function() {
                expect(returnValue).toBe(romanNumbers[propCopy]);
            });
        }).bind(this,prop));
    }
});

Review

What?! I thought we were done? Well, we are and we aren’t. We have code that works. But is it the best code we can write? Here is what I like about this code:

It is easy to reason about.
It allows me to add additional roman numbers by expanding my tables. No additional code would be needed. (And while it is hard to represent using Arabic letters, there are additional symbols.)
It works reasonably fast.

But, it does seem to me that we might make it a bit more efficient without sacrificing these advantages too much. And the great news is, since we have our test in place, we can refactor with impunity. No worries about breaking something because we will know as soon as we have and we can revert back to the code that was working.

Step 12

One pretty simple change we can make is that our error message is scattered throughout our code. Let’s make that a variable and just throw the variable.

The other thing I wonder about is how much of our validations can be combined? One check we might safely eliminate at this point is the check to make sure the minus values only show up once. If any of them were to show up more than once, they would either show up one after the other, which we still can check for, or they would be in the wrong value order which our final check will catch. So, let’s eliminate that code as well and re-run our tests to make sure nothing broke.

And now that we’ve eliminated that check, we can create one big string for checking conditions like “IVI” into one string and check it once instead of creating a new RegExp object multiple times.

Here is the code we have so far:

describe('tests/roman-to-arabic/RomanToArabic.spec.js',function(){
    var returnValue;
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    var baseTable = {
        I: 1,
        V: 5,
        X: 10,
        L: 50,
        C: 100,
        D: 500,
        M: 1000
    };

    describe('When I is passed in',function(){
        beforeEach(function(){
            returnValue = romanToArabic('I');
        });
        it('should return 1',function(){
            expect(returnValue).toBe(1);
        });
    });
    describe('When IV is passed in', function(){
        beforeEach(function(){
            returnValue = romanToArabic('IV');
        });
        it('should return 4',function(){
            expect(returnValue).toBe(4);
        });
    });
    var prop;
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + 'I' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy + 'I' + propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in baseTable){
        if(!baseTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy+propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        var newProp = prop + prop.substr(0,1);
        describe('When ' + newProp + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,newProp));
    }
    // just test for one each of upper case, lower case, number and lower case valid character
    var invalidCharacters = {A:1,a:1,'2': 1, i: 1};
    for(prop in invalidCharacters){
        if(!invalidCharacters.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    var badOrderPairs = {DM: 1, LC: 1, VX: 1, IXXL: 1, IXM: 1};
    for(prop in badOrderPairs){
        if(!badOrderPairs.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    var romanNumbers = {
        I: 1,
        II: 2,
        III: 3,
        IV: 4,
        V: 5,
        VI: 6,
        VII: 7,
        VIII: 8,
        IX: 9,
        X: 10,
        XI: 11,
        XII: 12,
        XIII: 13,
        XIV: 14,
        XV: 15,
        XVI: 16,
        XVII: 17,
        XVIII: 18,
        XIX: 19,
        XX: 20,
        XXI: 21,
        XXII: 22,
        XXIII: 23,
        XXIV: 24,
        XXV: 25,
        XXVI: 26,
        XXVII: 27,
        XXVIII: 28,
        XXIX: 29,
        XXX: 30,
        XL: 40,
        XLIV: 44,
        XLV: 45,
        XLVI: 46,
        L: 50,
        LIV: 54,
        LV: 55,
        LVI: 56,
        LX: 60,
        XC: 90,
        C: 100,
        CI: 101,
        CXI: 111,
        CMXCIX: 999,
        MDCLXVI: 1666

    };
    for(prop in romanNumbers){
        if(!romanNumbers.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            beforeEach(function(){
                returnValue = romanToArabic(propCopy);
            });
            it('should return ' + romanNumbers[propCopy] ,function() {
                expect(returnValue).toBe(romanNumbers[propCopy]);
            });
        }).bind(this,prop));
    }
});

Step 11

Now that we have our IVI check down to one string, it occurs to me that we can combine this check with our invalid character check. So, let’s do that next.

describe('tests/roman-to-arabic/RomanToArabic.spec.js',function(){
    var returnValue;
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    var baseTable = {
        I: 1,
        V: 5,
        X: 10,
        L: 50,
        C: 100,
        D: 500,
        M: 1000
    };

    describe('When I is passed in',function(){
        beforeEach(function(){
            returnValue = romanToArabic('I');
        });
        it('should return 1',function(){
            expect(returnValue).toBe(1);
        });
    });
    describe('When IV is passed in', function(){
        beforeEach(function(){
            returnValue = romanToArabic('IV');
        });
        it('should return 4',function(){
            expect(returnValue).toBe(4);
        });
    });
    var prop;
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + 'I' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy + 'I' + propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in baseTable){
        if(!baseTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy+propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        var newProp = prop + prop.substr(0,1);
        describe('When ' + newProp + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,newProp));
    }
    // just test for one each of upper case, lower case, number and lower case valid character
    var invalidCharacters = {A:1,a:1,'2': 1, i: 1};
    for(prop in invalidCharacters){
        if(!invalidCharacters.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    var badOrderPairs = {DM: 1, LC: 1, VX: 1, IXXL: 1, IXM: 1};
    for(prop in badOrderPairs){
        if(!badOrderPairs.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    var romanNumbers = {
        I: 1,
        II: 2,
        III: 3,
        IV: 4,
        V: 5,
        VI: 6,
        VII: 7,
        VIII: 8,
        IX: 9,
        X: 10,
        XI: 11,
        XII: 12,
        XIII: 13,
        XIV: 14,
        XV: 15,
        XVI: 16,
        XVII: 17,
        XVIII: 18,
        XIX: 19,
        XX: 20,
        XXI: 21,
        XXII: 22,
        XXIII: 23,
        XXIV: 24,
        XXV: 25,
        XXVI: 26,
        XXVII: 27,
        XXVIII: 28,
        XXIX: 29,
        XXX: 30,
        XL: 40,
        XLIV: 44,
        XLV: 45,
        XLVI: 46,
        L: 50,
        LIV: 54,
        LV: 55,
        LVI: 56,
        LX: 60,
        XC: 90,
        C: 100,
        CI: 101,
        CXI: 111,
        CMXCIX: 999,
        MDCLXVI: 1666

    };
    for(prop in romanNumbers){
        if(!romanNumbers.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            beforeEach(function(){
                returnValue = romanToArabic(propCopy);
            });
            it('should return ' + romanNumbers[propCopy] ,function() {
                expect(returnValue).toBe(romanNumbers[propCopy]);
            });
        }).bind(this,prop));
    }


});

Step 12

And the last place we can optimize is our three digit check. But instead of limiting it to valid Roman numbers, let’s just check for any sequence of characters that repeats 4 times or more.

And once we know that is working, we can combine it with our other Regular Expressions.

describe('tests/roman-to-arabic/RomanToArabic.spec.js',function(){
    var returnValue;
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    var baseTable = {
        I: 1,
        V: 5,
        X: 10,
        L: 50,
        C: 100,
        D: 500,
        M: 1000
    };

    describe('When I is passed in',function(){
        beforeEach(function(){
            returnValue = romanToArabic('I');
        });
        it('should return 1',function(){
            expect(returnValue).toBe(1);
        });
    });
    describe('When IV is passed in', function(){
        beforeEach(function(){
            returnValue = romanToArabic('IV');
        });
        it('should return 4',function(){
            expect(returnValue).toBe(4);
        });
    });
    var prop;
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + 'I' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy + 'I' + propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in baseTable){
        if(!baseTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy+propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        var newProp = prop + prop.substr(0,1);
        describe('When ' + newProp + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,newProp));
    }
    // just test for one each of upper case, lower case, number and lower case valid character
    var invalidCharacters = {A:1,a:1,'2': 1, i: 1};
    for(prop in invalidCharacters){
        if(!invalidCharacters.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    var badOrderPairs = {DM: 1, LC: 1, VX: 1, IXXL: 1, IXM: 1};
    for(prop in badOrderPairs){
        if(!badOrderPairs.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    var romanNumbers = {
        I: 1,
        II: 2,
        III: 3,
        IV: 4,
        V: 5,
        VI: 6,
        VII: 7,
        VIII: 8,
        IX: 9,
        X: 10,
        XI: 11,
        XII: 12,
        XIII: 13,
        XIV: 14,
        XV: 15,
        XVI: 16,
        XVII: 17,
        XVIII: 18,
        XIX: 19,
        XX: 20,
        XXI: 21,
        XXII: 22,
        XXIII: 23,
        XXIV: 24,
        XXV: 25,
        XXVI: 26,
        XXVII: 27,
        XXVIII: 28,
        XXIX: 29,
        XXX: 30,
        XL: 40,
        XLIV: 44,
        XLV: 45,
        XLVI: 46,
        L: 50,
        LIV: 54,
        LV: 55,
        LVI: 56,
        LX: 60,
        XC: 90,
        C: 100,
        CI: 101,
        CXI: 111,
        CMXCIX: 999,
        MDCLXVI: 1666

    };
    for(prop in romanNumbers){
        if(!romanNumbers.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            beforeEach(function(){
                returnValue = romanToArabic(propCopy);
            });
            it('should return ' + romanNumbers[propCopy] ,function() {
                expect(returnValue).toBe(romanNumbers[propCopy]);
            });
        }).bind(this,prop));
    }
});

Step 13

We’ve cleaned up about all of the logic we can without hard coding the values. But I would like to combine the lookup tables next. I don’t like having three tables. Let’s see if we can pull those into one table.

The first step to doing this is combining the minusOneTable and baseTable into one table. To differentiate in the code that is using those tables, we’ll just look at the length of the property.

In the process of doing this, we notice that it would also make sense to combine the substitution table. And since the values we’ve assigned to the table aren’t even being used at this point, we’ll just use the substitutions instead of the values.

And if we make the value of each element in the baseTable an array, we could combine the values into that table as well. But that would over complicate our lookup logic when we compute the value. So, I think we’ll just leave that as it is.

describe('tests/roman-to-arabic/RomanToArabic.spec.js',function(){
    var returnValue;
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    var baseTable = {
        I: 1,
        V: 5,
        X: 10,
        L: 50,
        C: 100,
        D: 500,
        M: 1000
    };

    describe('When I is passed in',function(){
        beforeEach(function(){
            returnValue = romanToArabic('I');
        });
        it('should return 1',function(){
            expect(returnValue).toBe(1);
        });
    });
    describe('When IV is passed in', function(){
        beforeEach(function(){
            returnValue = romanToArabic('IV');
        });
        it('should return 4',function(){
            expect(returnValue).toBe(4);
        });
    });
    var prop;
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + 'I' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy + 'I' + propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in baseTable){
        if(!baseTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy+propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        var newProp = prop + prop.substr(0,1);
        describe('When ' + newProp + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,newProp));
    }
    // just test for one each of upper case, lower case, number and lower case valid character
    var invalidCharacters = {A:1,a:1,'2': 1, i: 1};
    for(prop in invalidCharacters){
        if(!invalidCharacters.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    var badOrderPairs = {DM: 1, LC: 1, VX: 1, IXXL: 1, IXM: 1};
    for(prop in badOrderPairs){
        if(!badOrderPairs.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    var romanNumbers = {
        I: 1,
        II: 2,
        III: 3,
        IV: 4,
        V: 5,
        VI: 6,
        VII: 7,
        VIII: 8,
        IX: 9,
        X: 10,
        XI: 11,
        XII: 12,
        XIII: 13,
        XIV: 14,
        XV: 15,
        XVI: 16,
        XVII: 17,
        XVIII: 18,
        XIX: 19,
        XX: 20,
        XXI: 21,
        XXII: 22,
        XXIII: 23,
        XXIV: 24,
        XXV: 25,
        XXVI: 26,
        XXVII: 27,
        XXVIII: 28,
        XXIX: 29,
        XXX: 30,
        XL: 40,
        XLIV: 44,
        XLV: 45,
        XLVI: 46,
        L: 50,
        LIV: 54,
        LV: 55,
        LVI: 56,
        LX: 60,
        XC: 90,
        C: 100,
        CI: 101,
        CXI: 111,
        CMXCIX: 999,
        MDCLXVI: 1666

    };
    for(prop in romanNumbers){
        if(!romanNumbers.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            beforeEach(function(){
                returnValue = romanToArabic(propCopy);
            });
            it('should return ' + romanNumbers[propCopy] ,function() {
                expect(returnValue).toBe(romanNumbers[propCopy]);
            });
        }).bind(this,prop));
    }
});

Step 14

Finally, I think we want to consolidate the loops into one loop.

describe('tests/roman-to-arabic/RomanToArabic.spec.js',function(){
    var returnValue;
    var minusOneTable = {
        IV: 4,
        IX: 9,
        XL: 40,
        XC: 90,
        CD: 400,
        CM: 900
    };
    var baseTable = {
        I: 1,
        V: 5,
        X: 10,
        L: 50,
        C: 100,
        D: 500,
        M: 1000
    };

    describe('When I is passed in',function(){
        beforeEach(function(){
            returnValue = romanToArabic('I');
        });
        it('should return 1',function(){
            expect(returnValue).toBe(1);
        });
    });
    describe('When IV is passed in', function(){
        beforeEach(function(){
            returnValue = romanToArabic('IV');
        });
        it('should return 4',function(){
            expect(returnValue).toBe(4);
        });
    });
    var prop;
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + 'I' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy + 'I' + propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in baseTable){
        if(!baseTable.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + prop + prop + prop + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy+propCopy+propCopy+propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    for(prop in minusOneTable){
        if(!minusOneTable.hasOwnProperty(prop)){
            continue;
        }
        var newProp = prop + prop.substr(0,1);
        describe('When ' + newProp + ' is added',(function(propCopy){
            it('should throw an exception',function(){
                expect(function(){romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,newProp));
    }
    // just test for one each of upper case, lower case, number and lower case valid character
    var invalidCharacters = {A:1,a:1,'2': 1, i: 1};
    for(prop in invalidCharacters){
        if(!invalidCharacters.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    var badOrderPairs = {DM: 1, LC: 1, VX: 1, IXXL: 1, IXM: 1};
    for(prop in badOrderPairs){
        if(!badOrderPairs.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            it('should throw an exception',function() {
                expect(function () {romanToArabic(propCopy)}).toThrow();
            });
        }).bind(this,prop));
    }
    var romanNumbers = {
        I: 1,
        II: 2,
        III: 3,
        IV: 4,
        V: 5,
        VI: 6,
        VII: 7,
        VIII: 8,
        IX: 9,
        X: 10,
        XI: 11,
        XII: 12,
        XIII: 13,
        XIV: 14,
        XV: 15,
        XVI: 16,
        XVII: 17,
        XVIII: 18,
        XIX: 19,
        XX: 20,
        XXI: 21,
        XXII: 22,
        XXIII: 23,
        XXIV: 24,
        XXV: 25,
        XXVI: 26,
        XXVII: 27,
        XXVIII: 28,
        XXIX: 29,
        XXX: 30,
        XL: 40,
        XLIV: 44,
        XLV: 45,
        XLVI: 46,
        L: 50,
        LIV: 54,
        LV: 55,
        LVI: 56,
        LX: 60,
        XC: 90,
        C: 100,
        CI: 101,
        CXI: 111,
        CMXCIX: 999,
        MDCLXVI: 1666

    };
    for(prop in romanNumbers){
        if(!romanNumbers.hasOwnProperty(prop)){
            continue;
        }
        describe('When ' + prop + ' is added',(function(propCopy){
            beforeEach(function(){
                returnValue = romanToArabic(propCopy);
            });
            it('should return ' + romanNumbers[propCopy] ,function() {
                expect(returnValue).toBe(romanNumbers[propCopy]);
            });
        }).bind(this,prop));
    }
});

Review

There are probably a few other optimizations that could be made here. But I’m afraid that each would sacrifice either the flexibility. That is, we could hard code the regular expression validations, which would significantly reduce the number of lines of code. Or we would sacrifice the readability. Neither of which I am willing to do.

If you are only interested in the result, you can find the finished project at https://github.com/DaveMBush/JavaScriptKatas

7 Reasons To Evade Ext JS

2016-07-13

EXTjs

I’ve worked with Ext JS now for a total of 2.5 years. First with Ext 4.2 and now with Ext 6.x.

Here’s my experience, and warning, of why you should avoid this disaster of a framework.

7 Reasons To Evade Ext JS

Jack of All Trades

Master of none! One of the great selling points of using Ext JS is the fact that it comes with “Everything you need” to build a web application. That would be great if it were true. But the fact of the matter is, it comes with all of the features you need but the features are all only partially implemented. I’ve complained publicly several times that Sencha can’t possibly be testing the code they release because it only works in their demos. If you try to use a feature they have documented as being available, you are likely to find that the feature doesn’t actually work. How is it possible that you’ve written documentation for how something is supposed to work and yet you can release it without it working properly? I can understand fringe stuff getting by. We can’t think of every test. But when this happens over and over again, you start to wonder what exactly they are testing.

A Wolf in Sheep’s Clothing

When I first started with Ext, the only design pattern they had available was what they referred to as MVC. It took me two months of playing with the framework before I finally realized that what they were calling MVC wasn’t anything the Gang of Four would recognize as MVC. I guess if you have a View, a Model and a Controller, you can call it MVC? It doesn’t matter that the Models define records in a table or that the Controller is tightly coupled to your view.

Sheep Without Legs

OK. So when they introduced the MVVM architecture I actually started to have just a bit of hope. Yes, there were still some fundamental issues I have, but MVVM would make this tolerable. But here is the issue. Their idea of MVVM is that you would only need to implement it on a per page basis.

Let me try to explain.

Broken Data Binding

In my ideal world, when I build a new component, I would build that component using the framework the rest of my application is using. So my component uses MVVM. Sencha’s implementation gives you a View, ViewController, and ViewModel. Mostly this looks more like MVC if you ask me but whatever, it has two-way databinding, so we’ll call it MVVM for now. If you build a component that lives inside another component, the first thing you’ll discover is that binding only works from the top down. That is, I can bind data at the outer layer and it will get reflected all of the way in to the inner most component that uses it. But, if you change the data in the inner most component, it doesn’t reflect back up to the outer most component. I’ve written a hack for this, and there is no promise from Sencha that this will ever get fixed properly, so I guess my hack is safe.

Broken Controllers

But it gets worse. While child components can find data in models that are in parent components properly, they can’t find references to functions in controllers in the same way. This is particularly problematic if you write a component that is a container of other components. You would naturally want the child components to use the controller from the component that they were declared in. But if you have an outer component that has your container component as a child and then other components inside of that. The only way you can control what controller the child most components are going to notify of events is by wrapping the inner most components in their own component with their own controller. This gets to be awkward when all you want to do is provide an event handler for one control in a column of a grid control. Again, I have a monkey patch that fixes this, but why did I have to write it? This is just one specific example of my “Jack of All Trades” point that I started with.

We won’t even address the question of if this is really MVVM or not!

Never Use the .0 release

I think most of us now are generally conditioned to be wary of the .0 release of anything that hasn’t been developed using Open Source methods. There just haven’t been enough eyes on the project to ensure that everything works as it should.

But with Sencha, this extends to all of the patch releases at the very least and even into some minor releases.

While the 4.0, 5.0, and 6.0 releases were unacceptably broken, we find that every new patch or minor release that comes out afterward breaks something that was working. We always have to ask, “Can we live with this?”

All or Nothing

As I said at the beginning, Sencha gives you everything. That sounds good. You won’t have to go looking for a grid control, or many other common controls you might want to use.

But the bad news is, you can only use controls that were written to be used with Ext. Which other than what Sencha provides in the framework, doesn’t give you a lot of choices. Don’t go thinking you’ll supplement Ext with a selection of third party controls. It’s not going to happen.

Fences Protect AND Isolate

Up until this point in my post, no one can reasonably argue that anything I’ve said is actually a benefit. At this point we switch to points that may vary based on how well you know JavaScript, HTML, and CSS.

You see, the good news, and actually a major selling point to many people, is that you can write a web application using Ext without having to know much, if anything about HTML or CSS. And for that matter even the amount of JavaScript you need to know is relatively limited.

That’s the good news. The bad news is, if you know anything about any of these, you’ll probably end up frustrated by EXT. This is because Ext’s JavaScript controls most of the layout. So if you are used to going into developer tools to tweak the CSS and then applying that to your style sheet, you are going to be very disappointed. Pretty much nothing you do in developer tools is going to work as you would expect. And figuring out how to apply those to your code is going to be a lot harder than you are used to.

Their Way or the Highway

Once again, many people see this as an advantage. And once again if you aren’t familiar with how the rest of the JavaScript world does things, this is going to sound fine.

Sencha CMD

Everything runs through Sencha CMD. A tool for building all things Ext. If you want to bundle and minify your code, the standard way of doing this is by using “requires” statements in your code and then running Sencha CMD and have it figure out what you are using and put it all in one bundle.

The problem with this is that there are several much better ways of doing this that are available using Node and various NPM packages. Again, if you are a JavaScript developer, you are going to wonder what Sencha is thinking.

Ext.define()

Another place where proprietary shows up is in how Ext defines “Classes”. When it was first introduced, TypeScript was new. But now, we not only have TypeScript, which does much of what Ext does and some things it doesn’t, but we have an evolving JavaScript standard that I’m afraid Sencha won’t be able to keep up with. They already discourage the use of ‘use strict’;. Once again, there is only one place where this will get you in trouble, and the work around actually produces more efficient code. But still, the point is, Sencha is relying on ECMA Script 3 standards while the world has largely moved on to ECMA 2015 and beyond.

Anyhow, my point here is that Ext is not just a framework but also functions, largely, as its own language. Not quite as much a fork from the standard as Coffee Script, but also not nearly as close to the JavaScript spec as TypeScript. So while it is still JavaScript, if you are a JavaScript programmer, it isn’t going to feel quite like JavaScript to you.

Themes

The final place you will find “Proprietary” lurking is with the Themes. There are several really good CSS frameworks out there. Sencha uses none of them. And while the syntax they use for creating themes has been SASS up until Ext 6, now they even have their own proprietary SASS compiler. Watch out here because they are still using the SASS extensions so you are likely to make some assumptions here that aren’t true because, once again, they’ve only implemented enough of the SASS engine to do what THEY need to do.

VB All Over Again

Every time I hear someone praise how great Ext is, it is normally because it has everything you need out of the box and allows you to get stuff done quickly.

Basically the same argument for using Visual Basic back in the day. And yet I learned to never take a VB job because it almost every instance, while it was possible to write well structured code in Visual Basic, it was generally so difficult to do that the code I would be maintaining would need to be rewritten in order to make any sense of it. Ext suffers the same issue. There is nothing in Ext to force you to write well structured code. The code I have had to maintain has almost always followed every anti-pattern known to man. In this case, this isn’t Sencha’s fault directly other than the fact that the only reason my code tends to be cleaner than most is because I’m more likely to code a fix to an Ext bug than I am to work around the problem with an anti-pattern.

In comparison to other frameworks that are available, if all you want is a tool that will get you a semi working application quickly, and you don’t care so much about having to rewrite it when you need to change it in some way, Ext is your tool. If on the other hand, you care about design and you want to be able to maintain what you’ve written, you should look elsewhere.

Remember, if it sounds too good to be true, it probably is.

Dave's Notebook

History of MVVM

Definition of MVVM

MVVM Done Right is Slow

MVVM is Hard to Implement

MVVM is Hard to Reason About

MVVM is Overkill

Where to Go from Here

Other places talking about MVVM

Today’s problem:

Test 1

Test 2

Test 3

Test 4

Test 5

Test 6

Step 7

Step 8

Step 9

Review

Step 12

Step 11

Step 12

Step 13

Step 14

Review

Jack of All Trades

A Wolf in Sheep’s Clothing

Sheep Without Legs

Broken Data Binding

Broken Controllers

Never Use the .0 release

All or Nothing

Fences Protect AND Isolate

Their Way or the Highway

Sencha CMD

Ext.define()

Themes

VB All Over Again

Recents

Archives