Dangling underscores

A pull request at work sparked a bit of conversation[1] about using underscores in JavaScript to describe a function as being private. It was this code specifically:

Because our logic isn’t as separated from our database as we’d like, many of our API tests touch the ES database. In order to avoid futsing up our actually data, we just want to make sure it’s only ever talking to indexes starting with “test_”.

My suggestion was that we should refactor this so that we don’t need to be exposing what should be a private method.

The argument was that this is a private method; it’s convention in JS to just use this underscore syntax. But that argument really just comes down to “that’s the way it’s always been done.”

This was fine back in the dark days of JavaScript, but we’re much more organised these days. And as our industry grows more mature it’s our responsibility to keep up with it.

Leaving something public really limits how much you can change later – regardless of it being an “underscore private method” you’re still letting others see it, and you’ve no way to know that it’s safe to delete that method once you’ve changed your implementation.

Even if you know that’s not a concern,[2] it’s still a good habit to get out of. And a free one at that.

[1] Which had nothing to do with the feature set being added, as always seems to be the case: comment on the low hanging fruit, rather than the structure or flow of the code.

[2] In this example, the proxy probably won’t be used again and has no need to be extended.

Om nom nom: Pepper Spring Greens with Buttered Cashews

Those that know me know I’m not incredibly into food; despite that I decided to sign up for an Abel & Cole veg box, and try and cook something. This is probably third or forth time I’ve ever tried to cook from a recipe!

This is the original recipe, from the Abel and Cole veg box companion book.
This is the original recipe, from the Abel and Cole veg box companion book.

In our box recently we received Spring Greens, which are a lot like chard, apparently, and Tim wanted some cachew nuts so we went with this recipe.

Another alteration we made was (because we couldn’t find any in Morrisons) pepper instead of chilli. Quite different, but worked really well. It was another ingrediant from our veg box, so I was pretty chuffed to be using them up.

IMG_1096First task was for me to start slicing up some onions. I’ve no idea what sliced means so I just cut them up a bit – horizontally and vertically. That’s probably fine.

Sliced onions
Not actually how I was cutting them. I’m not a freestyling knife ninja. Tim wanted an “action shot” of the cutting, but my hand was in the way.

Meanwhile, Tim was busy toasting the rice. We figured that meant “burn it a little”. We were very curious about this, as burning food was often the opposite of what the recipe tells us to do. We browned it off quite well, I think. It’s resistance to burning might have something do with the olive oil we generously gulped in.

"What, burn it? I can handle that."
“What, burn it? I can handle that.”

Next we added two cups of water to it, and it smelled really nice. Not like burnt food at all, but something we quite wanted to shove in our faces.

This is a technique Abel and Cole seem to love – “cups” and “gulps” and “handfuls”. Not actual measurements. Put Tim and I right out of our computer science comfort zones, but it all seemed to work out!

Another sensual action shot.
Another sensual action shot.

We went with the whole pepper, even though it was supposed to be substituting a single chilli. I like pepper, so we figured we’d go wild.

We were to wait until the onions were glossy. I’ve no idea how glossy onions are supposed to get, so we just went with it. I’ve learnt cooking is a lot about “yuh – that’s probably looks done”. How liberating. At that point we threw in some garlic and the peppers. And more olive oil, of course.

Peppers and onion, into the pan.
Looked and smelled pretty tasty pretty quickly!

We almost forgot about the spring greens and threw it in at the last minute. We were umming and ahhing over if we should be chopping it up or not, but decided that if it’s anything like spinach it’ll shrivel up when it hits the hit. It did a little bit, but we were probably supposed to tear it up as we were putting it in the pan.

Spring greens, into the pan!
The leaves being folded into the peppers, garlic, and onion.

By this time the water had boiled off the rice and it seemed to have steamed itself happily.

“Steam the rice” was a confusing command for us. We actually bought a steamer (was £15 at Morrisons, so we thought it was worth the punt) but decided against using it. It looked like the rice was full of steam anyway, since we had the lid on the pan. We almost never use that lid, so it was nice to get that out of the cupboard.

Nicely browned rice.
Nicely browned rice.

Because we were late throwing in the greens there was a worry that the rice would go cold before everything else was done. Not sure what we could do about that, but it turned out to be fine anyway.

Finishing up just meant frying the cashew nuts, which we messed up a little. We didn’t realise how quickly they would burn, and so ended up a little chard (no pun intended…).

Completed spring greens and toasted rice

It looked nothing like what was in the recipe book, but we were never expecting it too. It was really delicious though. I wasn’t in high hopes about liking it at all – I was actually mentally preparing a back-up meal for if this was foul. But we ate every bit of it.

As you can see, I took the excuse to break out my camera and catalogue our cooking experience. I was using my Canon 60D with a 50mm lens. Really narrow focal range, which can be tricky to work with, but really good at giving emphasis. Maybe my favourite lens I’ve played with.

I enjoyed not having too much time to fuss around with the shot. We were busy cooking! I had to shot and go.

On keeping git clean for pull requests

There’s lots of places that you can find talking about how to write a good commit message. Some will even give good technical reasons for doing so, but only a few will give you actually good reasons. When’s the last time you had to use git log --oneline or send a commit via email? Maybe Linus does this everyday, but you don’t.

The one reason I find most important when you’re thinking about the git history as a whole (not just on a per-commit basis) is during peer review. In my current work place and my last the way we did code reviews was by a collogue pushing to github, and opening a pull request. It was then someone else’s job to review and pull that in. That seems to be a very popular workflow from talking to a number of tech teams the past few months whilst interviewing.

Forget about being easily able to look back in the project’s history and see why something was changed, or being able to see a good message on git bisect. They’ll come incidentally with the following method.

Review is the one stage in product development when you’ll be reading commit messages the most, but it seems like no commit messages I’ve encountered recently, at the very least on my team, are written for that part of the process.

Failing to describe the scope of the message

The first thing I want from a commit message is to know the mindset I should be in whilst reviewing. What are you trying to achieve with the code change you’ve introduced? This Doctrine commit message could be slightly better by saying “always add ‘isEmbeddedClass’ to the serialise array”. I could look at the commit message, and at the code change and see “yup, this is doing exactly what the author wanted it to do.” From that point, I can start thinking of side affects the code change might have on edge cases.

The problem with “fix bug” is that I can’t tell what issue with the code you’re talking about, and so the only review I can do is “yep, that’s syntactically correct”. I can still think about potential side affects, but I’ve no idea if the original writer has thought about and accepts them.

Splitting one unit of work across too many commits

A unit of work should be valuable in some way. A commit should hold one unit of work. If we were to revert this one commit from your pull request, how many other commits would have to be reverted too before we got to a working piece of code? It should be as few as possible without affecting other features.

This usually happens when you’re working in stages and commit each stage. That’s fine at giving you good rollback points, but they should be merged together later on when you’re self-reviewing. An example of this is in the BrowserQuest repo, two commits right after each other with the same commit message: ae773f and 9c1d5.

A more common (and more annoying) issue is when a file has been forgotten to be added, and so the second commit is “lol, forgot to add this file”.

These are exactly what  --amend flag to git was added for; instead of adding a new commit, just push the changes into the last commit. Slightly more complicatedly, --fixup exists too.

Never mix styling or whitespace changes with functional changes

Most teams with an old codebase have the idea to clean as they go along – that’s a good idea (rather than a huge, up front investment).

These commits should be on their own, because I’m not even going to read them. Your team mates should just be able to auto-merge these commits in (especially in a language like PHP where whitespace doesn’t matter).

The problem is that if you’re changing from tabs to spaces, the git diff will say that every line has changed. Your functional changes will get buried and it won’t be possible to see if anything will have unexpected side effects.

On recruiters not being partularly helpful

As I was looking for a job this time around, I had largely the same experience with recruiters as I did last time around. Although, with a lot more interest.

I posted my CV online on jobsite.co.uk once again, just like last time. But instead of just the ten or fifteen phonecalls the next morning, my phone was ringingly solidly at least the first two days.

This was just before 10:30, and I'd been answering the phone all that morning until then too.
This was just before 10:30, and I’d been answering the phone all that morning until then too.

I can only conclude that the market for web developers has only gotten better when we’re looking for a job.

Once again, I had intreviews line up within hours. One interviewer asked if I was available for an interview “in a couple of hours”. There’s no doubt that you’ll find a job via a recruiter.

On the other hand though, there’s lots of baggage that comes with it. They’ll often get upset and unprofessional if you turn down an offer – “you’re being really irresponsible turning down this opertunity, you won’t get another one like it”, I was told by one recruiter. When they’re not incredibly unprofessional, they’re pushy. These are the times when you realise that the majority of recruiters don’t care about how happy you are – they only care that you find a job.

I should mention that not every recruiter I worked with this year has been awful. Maria from ASG was wonderful at her job, and I really feel like she didn’t feel too pressured into making a sale. I really felt like I had a nice bond with her – she understood what I was looking for. She’s a Symfony specific recruiter.

If you’re going to go with recruiters, definitely go with only one. Don’t publish your CV on a job board and expect anything other than to be treated like high value meat. Find one that your friends have recommended. There are some trustworthy ones out there.

In the end though, much like how I joined MyBuilder, I got my new position by applying directly. I very much recommend this route. Look at who’s sponsoring your local meetups – they’re probably a cool company to be working for. I found my new company – Altmetric – through Siliconmilk Roundabout.

But my point is, the hassle and stress that came with recruiters wasn’t very fruitful ultimately.

On Let’s Code JavaScript

I’m really enjoying Let’s Code Test Driven JavaScript, from James Shore.

His aim is to release videos overtime, showing how he’s developing an arbitrary application. Using JavaScript and Nodejs and driving the development with tests. Not just the code, but the set up as well.

The videos also come with DRM free download links.
The videos also come with DRM free download links.

It’s subscription based, but with a free trial (all access to the content, for seven days). I discovered this whilst in-between jobs and had no income, so in a time where money is heavily budgeted it’s a testament to James’s work that I decided it was worth the money.

In fact, I think it’s so important to the dev space that I’ve linked to LCTDJS in what I usually reserve for ad space on this blog. You guys need to be watching these videos, even if you already know how to write node apps – the test driven component is brilliant and getting to hear some of James’ ideas should make this a really compelling subscription.

The videos are fairly short. I think they’ve been designed that way so they’re commute friendly. I can’t imagine sitting on a train watching these videos, but maybe you can. The shortness is pretty helpful for me since I’ve now got something to watch whilst eating that’s actually productive (instead of endless Youtube). Despite being short, I’ve not yet watched an episode and thought “well, that was a waste of ten minutes”. I’ve always walked away with a new idea to think about. And there’s such a backlog of videos for me to catch up on, if you need more just watch another.

The videos manage to kick off at a really good point – assuming you know how to program. What you’re learning really is the methodology behind programming (the thing that good programmers focus on). Where you should be spending your time. The code is often secondary to how well it’s tested. The best practices talked about in the series really should be sitting along side industry standards set out in Clean Code.

The videos start by adding build automation and linting. Nothing to do with getting the app working, just making it work cleanly. The emphasis on this throughout the series is why I keep watching. I don’t care to watch a guy telling me the problems and features needed in a collaborative painting app. I care about how the code is designed, and how to think about the choices that come up.

The chapter I’m on at the moment is about deploying the code. It’s not something that would have been consider a software developers job in the past, but now people in our job are going to find themselves more often in the command line. Faster deployment means developers are the ones pushing to live, not administrators. So it’s interesting to see how you can wrap tests around deployment. Had you every thought about TDD for things that aren’t strictly code? Because you should be.

My favourite aspect of the series is the warts and all approach. It would have been easy for James to have edited the videos in a way that just shows how to do TDD. But he also leaves in the walls he runs into. My favourite episode so far is episode 31. He starts the episode realising what he’s worked on recently is a waste of time, and then ends it by running straight into a bug. This is exactly how I program. More than likely how you do to. It’s amazing to see that these problems aren’t a reflection on me as a programmer, but they’re just part of the daily trials of every programmer.