Step 7: Interlude & Refactor

We've hit our first real milestone with our application. After a whole lot of blood, sweat, and imaginary tears we have mostly finished a mostly stable accounts module. There is no doubt in my mind that we will add or change it later, but for now it is fully functional. Time to go outside and take a breath of fresh air.

A few things have been nagging at me while coding this module. In fact, I tend to keep a notebook on my desk in which I jot notes and tasks as I code. I highly, highly recommend this practice. It's sort of the analog equivalent of littering your code with TODO comments everywhere (which I still do sometimes!), except everything is in one place and it is more flexible. I also use a different section of the notebook to make notes, low-fi sketches, or work through complicated problems. Use a spiral notebook so it stays flat and open all the time.

Photo of my programmer's notebook and pen

These things that have been gnawing on me are all written down in my notebook. At this point I usually take them and turn them into tasks or bug reports in my project management system. Personally, I use Kanboard because it's extremely powerful and flexible. This ensures that they don't slip out of mind. One item in particular has been driving me crazy though, and that is my inconsistent usage of create/add, edit/update, and remove/delete.

I try to stick firmly to CRUD terminology, and here I have failed. Now, you can call your methods whatever you feel most comfortable with. If you prefer add instead of create then roll with it. The problem is that I keep switching back and forth. I call my component CreateEditAccount while the actions are addAccount and updateAccount. In my deleteAccount action I call a removeAccount API method.

(I draw the line at read, as it is not descriptive enough for my liking. I could accept retrieve, but prefer load when populating data and fetch when actually grabbing it from a server or database.)

This might feel like a trivial problem at this point, but when you are working on this code 3 weeks from now it will annoy you to no end that you can't remember if you called a method edit or update. What's worse is that we are about to dive into our next module and the inconsistencies will only grow from here. So this is a great time to make this small refactor. There is no need for me to walk through the code here, as nothing is fundamentally changing.

At the same time, I'm also going to remove View from all of my list components. That's a habit I carried over from Django, but I feel it is redundant to call it AccountsListView.vue since every component is in some sense a view file. It might be nice to somehow differentiate between page components like our accounts list, "partial components like a navigation bar, and helper components like a datepicker, but in this application I don't foresee that being a big problem. Besides, we can probably tackle it through our project structure rather than our naming schema.

Now that is through, look through the code for any other pieces that you think need refactored. If you're not happy with any code make a task to fix it later, or go back and fix it immediately. Anything you look at now and can't understand immediately may need to be commented on or refactored. One line popped out to me in our actions.js file.

Object.keys(res).forEach((key) => { accounts[res[key].id] = res[key]; });

This is a harmless one-liner that converts the data returned from our API to the format we need for the store. It's sort of ugly, but not the worst I have seen (or written). When I look at this line after a few weeks will I realize right away what it does? I could probably reason through it but its purpose isn't immediately obvious. We could just add a comment to it. But we're going to have to perform a similar task for every type of data we call from our database. Maybe we should turn it into a utility function somewhere.

Now it begs the question - does this belong in our API or in our actions? Should actions be manipulating data in unexpected ways before storing it? Should our API always give us data in the format in which we need it?

These questions are a matter of programming principles rather than actual functionality. In reality, in this circumstance, it doesn't really matter where this line of code belongs. But when your application becomes more complex and the single line of code grows into 40 and you are hunting down a critical bug causing intermittent data loss because of a formatting issue and you have run a hundred tests on your API code and can't figure out the problem and you waste hours of your time before realizing that the application is altering the data in the Vuex action and the bug actually lies over there, suddenly this seemingly trivial problem has real consequences.

After a lot of back and forth I decided to move the processing code into my src/utils.js file since I will be using it in multiple modules. If I end up writing more global API functions I will create a src/api.js file, but right now it isn't needed. I then moved the actual conversion into my API code. The API layer we are writing should return usable data, so that felt like the best place to me.

import localforage from 'localforage';
import { processAPIData } from '../../utils';

const ACCOUNT_NAMESPACE = 'ACCOUNT-';

export const fetchAccounts = () => {
  return localforage.startsWith(ACCOUNT_NAMESPACE).then((res) => {
    return processAPIData(res);
  });
};

...
...

export const processAPIData = function (data) {
  /*
  Converts the data formatted for IndexedDB / API into the format
  our application uses.
   */
  let res = {};
  Object.keys(data).forEach((key) => { res[data[key].id] = data[key]; });
  return res;
};
...

export const loadAccounts = (state) => {
  // loads accounts only if they are not already loaded
  if (!state.accounts || Object.keys(state.accounts).length === 0) {
    return fetchAccounts().then((res) => {
      state.commit('LOAD_ACCOUNTS', res);
    });
  }
};

Finally, I discovered a pernicious little bug. When you edit an account its balance disappears! This is why you should write a test suite for your code! It would have caught this issue right away. Looking at the code, this happens because I decided to only load part of the account object when updating in CreateUpdateAccount.vue, yet our save code overwrites the entire object rather than updating only the fields that change. This is something I should have given more thought at the time. In this case we will switch to loading all of the object's data in our component and only expose the fields to the user that we want them to update.

// src/app/accounts/components/CreateUpdateAccount.vue

...

if (selectedAccount) {
  this.editing = true;
  this.selectedAccount = Object.assign({}, selectedAccount); // we copy to a new object so we aren't editing the real object in the Vuex store
}
...

324fbe9

23182d1

Continue to step 8, Budgeting


Originally published on


Sign up to receive updates for new articles.
And spam. Definitely lots of spam.