0

tl;dr - On an existing angularjs grid library, am moving to another. Is it better to wipe out as much code as possible and in a sense code from scratch to the same featureset, or work around what's there? Is there a recommended way (or even a good idea) to try to leverage what's there?

Better context:

My team has inherited an existing web app, running in production and used by consumers. The main component & featureset is around a table / grid, there are bulk edits, inline edits, actions per row etc. The code is not fantastic, it has lots of on $scope global variables as the grid interacts with filters and graphs on the page. The test coverage is middling, we have added unit & functional tests to give us more confidence in making changes, but it is not 100%.

At a corporate level, a new library for grids has been chosen in the past few months, to standardise the behaviour as well as look & feel of all the different teams using different grid libraries.

We're close to starting the move and I'm looking for thoughts on how best we should proceed. Do we wipe away the main controllers & services? Keep the unit tests and use them as feature specs?

Or piece by piece refactor and leave the old code in files next to the new ones?

  • Do you have end to end tests on this? Like selenium tests? – Luc Franken Jul 13 '16 at 19:28
  • Yes, we have written some protractor functional and smoke tests. I just don't think it's all encompassing. – Gavin Fitzgerald Jul 14 '16 at 9:51
0

I would go per feature. So take every issue (bulk edits, inline edits, actions) as a separate task. Then I would setup a new page, so both can be functional at the same time, with the new library.

Basic steps

Per issue I would do the following:

  • Review the end-to-end tests and check if they are complete so all functionality is covered.

  • Redirect the end-to-end tests for this issue to the new page (so now they are broken). You can do this one at a time off course. If you want you can even duplicate them so you keep running them both at the same time for the new and old implementation.

  • Make the new feature working so your tests start passing.

  • Review the relevant unit tests for this feature and duplicate them to test the new code. For example if you have a event handler being tested with the unit tests: Decide where the business logic should live, likely in an object. So, copy the rule but now test the object. Again you can keep the old ones running but mark them so you know that you have integrated this business rule in the new solution.

In this there are a lot of decisions to make like:

Decisions to make

  • Is the client side code implemented in a separate object or just inline event handlers? If inline I would move them to an object first, then let the new grid call the object. So you get separated responsibilities. Like a bulk edit object.

  • I would NOT try to touch all layers at the same time. If the controllers are fine just leave them for now. In case you need to change them you are still safe because the end-to-end tests you checked.

  • If inline it's likely not well-tested. Create (and re-use) tests to ensure the functionality now is stable.

  • Review layout issues/changes because the new grid might look different.

  • Consider updating manuals / screenshots.

When done you can return to the old url when preferred.

Next time it will be easier to replace the grid because it's quite separated.

Example on unit test:

Old:

Test.add('Add row', function (test) {
  var Grid = newGrid(3); // get grid with 3 rows
  Grid.events.trigger('addRow'); // Old event handler call
  // Can even be outside the object on a global, more ugly:
  // Events.call('grid1', 'addRow');
  test.equals(4, Grid.length);
});

New:

Test.add('Add row', function (test) {
  var Grid = newGrid(3);
  Grid.addRow(); // new event handler call
  test.equals(4, Grid.length);
});

No need to remove all unit tests. Just change the relevant pieces in your new copy should be sufficient. Clearly you will have old tests which become irrelevant and you will need to add new tests where new code is written.

  • Hmm, will do a full review of the end-to-end tests and identify the vital gaps, thanks. Thoughts on keeping the unit tests, but removing the files they're testing? Is it too granular and likely to keep the same problematic code? I just think there's a significant amount of business logic in them. – Gavin Fitzgerald Jul 14 '16 at 13:43
  • Once we do the first change of the table, 300+ unit tests fail. Should they be disabled and examined as we re-add each new feature? I am trying to split the work, feature by feature, smaller completable chunks. – Gavin Fitzgerald Jul 14 '16 at 13:51
  • My strategy would keep the old url fully functional. Constantly. At a new url you build the new grid. Only when you are ready you switch over. In between you can even do urgent bug fixes on the old one and deploy them without issues. So I don't say: "Change the table" but I say: "Build a new one" by implementing the features piece by piece. And code ready for removal, like old event handlers, old unit tests etc are being marked in the process. So in the end you have all tests running 100% (you get > double the amount of tests). And then at that moment when accepted you remove old (marked) code – Luc Franken Jul 14 '16 at 14:15
  • Ah yes. The current plan is to branch off develop and having a living alternative to it, that has the new table. Because we will need to still contribute fixes occasionally. Currently thinking we would empty the unit tests files of code, keep the descriptions of each (which are written in some what of a Given, When, Then style) and as each developer works on part that relates, making sure those are covered. – Gavin Fitzgerald Jul 14 '16 at 14:24
  • Certainly branch off off course. The advantage on the way I sketch you is that the developer can just open 2 browsers on the same instance next to each other. They work on the same instance, can directly compare with the same data. That in general improves efficiency. I would not empty the unit tests. You cannot trust just the description and it will be way more work. Will add an example on that. – Luc Franken Jul 14 '16 at 14:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.