Tuesday, February 16, 2021

In Coding, Nothing is Trivial

Imagine you're reviewing some code, as part of a Pull Request, and the developer has misspelt a variable. Or added a magic string instead of a constant. Or put the expected/actual parameters in an assertion the wrong way around (although it kinda doesn't matter because they're the same)?

You can politely comment on it, and perhaps they will fix it up. But sometimes you might be met with resistance. 

"It's not that important."

To a degree, they're right. These are pretty trivial observations. There are probably bigger things to address, such as the logic, the architecture, the test coverage etc. 

However, in coding, these trivial issues tend to come back to bite you, either as bugs, or as maintenance headaches.

You don't want to nag people. You don't want to become a "clean code nazi". But you can firmly assert that you think a mistake is being made and that a potential issue could arise. 

There are some things in coding that are a personal preference. There are some things that are virtually (though not completely) insignificant and which you should avoid fighting about (tabs vs spaces) and then there are things which actually matter. The lines are blurred and you should always be pragmatic - but the things which actually matter can be proven to matter - a misnamed variable can cause confusion, expected/actual parameters the wrong way around can lead to false positives in your tests, and magic strings can lead to runtime errors, code duplication, and mysterious bugs that can really mess up your day.

What's important is that you communicate these issues when making your suggestions. Make it clear that this isn't a personal preference thing - it's not you being fussy, but that it could lead to genuine problems. 

The main reason that many of these "trivial" issues can lead to serious problems is maintainability - other developers working on the code.  Maintainability is just so important - not having it leads to bugs, slow development, demoralised developers and more. Anything that affects maintainability, isn't trivial. Its effects are amplified because of all the people that become involved in the problem.

When you have maintainability problems, you get bugs. It doesn't take much to turn a badly named variable into a half day investigation and a costly Production change.

So, raise your standards. Don't settle for "It's not that important". If you have an issue with it, if it niggles at your brain even slightly, it's for a good reason. Your subconcious can spot a time bomb. Trust it.

Tuesday, November 24, 2020

Writing Good Acceptance Tests

Acceptance tests can be used to verify end to end functionality in a way that is understandable to the business. They are very broad compared with unit tests, and when automated via a tool like Specflow, are based on a more natural language. It's important that we use this natural language to keep tests simple and concise, rather than trying to write long-winded "functional" tests.

Good Gherkin

Before writing Acceptance tests, learn about Acceptance Criteria Best Practices. The ability to write good Acceptance Criteria translates directly to Acceptance testing. They both follow the same standards and they can be written in the same way.

We should attempt to apply these best practices apply to Acceptance tests as much as is practically possible, especially when it comes to grammar as this helps us stick to a standard.

The most important thing is to remember that Acceptance tests should be written with the Given, When, Then syntax where:
  • Given sets up a state,
  • When is an action,
  • Then is an assertion.
Every test should have an action and an assertion, and most require some kind of prerequisites. This ensures that the outcome of actions are actually being tested, and maps directly to a test case.

Let's reiterate a couple of points.

Given should be used to set up pre-requisites. It should not be used for actions, for example "Given I navigate to the admin page". You can however say "Given I am on the admin page" which achieves the same thing but keeps the structure clearer.

Likewise, Then should not be used for actions, such as "Then I delete a record". Then is for assertions, for checking the state of something that was changed by the "When" directive.

Remember to be careful when using AND that you don't break these rules. For example, don't say

"WHEN I delete a record THEN I should have a record AND I click exit"

In this case, the AND is now a "When", not a "Then". In other words, AND should be the same as the preceding directive. In the above example, as it is not an assertion, it should be a "When", and you probably want to follow it with another assertion "Then".

It might sound like I'm picking on grammar here but the point is that the 3 directives serve different purposes - the point of a test is to ensure that an action has an outcome. Mixing up their use means that you may not be creating an actual test case, but rather running through a series of steps, and you may not even have any assertions.

Focus on business actions rather than functional sequences

Practically speaking, in many Acceptance tests there are usually a lot of When steps, or actions performed. However, where possible we should strive to make all End to End code as concise as possible. Writing Specflow/Cypress that does several steps on one line is OK if each of those steps make up a single functional action.

For example, rather than 
When I populate the form with the following values: | Field | Value | And I click the "Save Changes" button

We could just say
When I save the following values:| Field | Value |

and have the underlying code also click the save button/save the changes.

The best way to think about this is we should be more concerned with business actions, or feature functionality, than stepping through a sequence of button clicks. 

Avoid multiple When/Then repetitions

One test, one behaviour, where possible. Any more and you're really testing multiple criteria, and each test should be testing one scenario (hence the Specflow keyword "Scenario").

Of course with Acceptance tests having such a high overhead, you might still want to do multiple assertions from time to time, but use it sparingly and be aware of it. 

Use Background steps

A background step happens before every Scenario in a feature file. Use it for setting up data. Then your tests can be clean and start with a simple "Given I am on the Search screen".

If you need each scenario to have different data you can put the data setup Given steps in each test, but in this case you might want to consider why there is different data for each test, and perhaps you need to move the tests into a different feature file.

Avoid testing logic

It can be tempting to test lots of different scenarios with Acceptance tests. I am speaking from extensive experience when I say: this will make your life very difficult. Acceptance tests have high overhead and can be extremely flaky.

Acceptance tests are for testing high level End to End functionality. They should not be used for testing intricate business logic. This should be done by Unit and Integration tests.

If it is too hard to write Unit/Integration tests, you may need to look at making your code more testable and addressing technical debt.

A large part of avoiding testing the logic comes down to testers and developers working together. When this happens, you can avoid a situation where the testers may write a dozen Acceptance tests which just test the same code path with different variables - not valuable from a quality perspective and also adding a huge unnecessary overhead to the testing time and maintenance. 

When working together, developers and testers can come up with a strategy where the logic and edge cases are covered by unit/integration tests, while the basic Acceptance Criteria/end to end connectivity/functionality can be verified by a small number of Acceptance tests. 

Don't write too many Acceptance tests

Acceptance tests are known for being flaky and unreliable. They can also be hard to maintain. Keeping the number of tests low makes them easier to manage. 

Acceptance tests can provide tremendous value by verifying End to End functionality, but people need to be able to trust them, so keeping them easy to maintain is key to keeping them reliable.

Wednesday, May 08, 2019

Clean coding Tips: Access Modifiers

It seems to be by default that classes, methods, etc are created with the public access modifier.

This is not actually the case, the default is internal class/private members, but public seems to be the de-facto choice by a lot of developers.

If you're creating a public class - you have a big responsibility. Your code can now be used by anything else in the application, or even outside it. This means if you change the internal working of it, you will affect everything that uses it - and you have no control over what that is. This means a couple of things.

Naming is always super important

Naming of public classes and methods and their expectations should be crystal clear. Don't rely on comments to convey intentions. It must be highly obvious what the class is for, what it does, and how method parameters are used. It should be specific and adhere to the Single Responsibility Principle - one reason to change. If someone comes along and adds a parameter to one of your methods to get it to do something else, this is an indication that the class's purpose was not clear enough.

If the naming is not clear, the purpose gets diluted, and the class will be modified away from its original intentions. This causes code to bloat and become more prone to bugs.

Tip: This is yet another reason to avoid "generic static helpers"! If your classes or methods are public, tests need to cover every possible way that the code can be used - every single path. What this means is that anyone using the code will know exactly what is expected of it because its functionality is documented by tests. Any usage of the code will already be covered by a test.

If you don't create tests, anyone can come along and change this code which can then affect any number of consumers.

Don't make life hard for yourself

Don't want this responsibility? Simple: Don't make your classes and methods public. Of course internal members should always be tested and code should be named well. But these issues take on less importance if the code is isolated.

Go with the lowest access first and only open it up as and when you need to, When you do open it up, make sure that it does what it says it will, and it is fully tested. The important thing to remember is that when a class is public, it increases its exposure, which allows it to be coupled with more code. This is a significant cause of spaghetti code and regression issues as the code is maintained.

Keep code restricted to where it is needed. If it needs to be public, it needs to be robust.

Friday, October 05, 2018

Why should I care about Quality? I'm a developer!

OK, no developer wants to create bad quality code. But for most developers, quality is seen as something separate from development, something that is checked after the work has been done. It is something that QA do, after you throw your code over the fence. Thinking about quality at during development is a burden, slowing down the creation of functionality, holding up new features. Thinking about quality before development, well that's just crazy talk.

Old Habits Die Hard

In the Agile world, this is starting to change. Quality is being baked into the development process and there is more interaction between development and QA during the development phase. At least, that's the theory. In practice, we often slip back into bad habits of throwing code over the fence.

The reason we keep falling back into bad habits is because the attitude around quality is the same as it always was. It is still seen as something separate from development, separate from design, separate from planning. This causes it to be seen as a burden - extra work that just has to be done.

Quality as the Glue

If instead we change our perspective, and try to see Quality as an integral part of the entire development process, it can change the way we all work, from developers to product owners. Instead of a burden, quality should be a thread that runs throughout the process, and actually defines how all work is done from the ground up. If this is done, it can actually simplify and speed up development.

Baking quality into your process shifts the responsibility to everyone, rather than just QA. Having the whole team (including product owners) take responsibility for quality sets the focus around Acceptance Criteria. This "requirements-centric" approach gets everyone thinking about quality right from the refinement meeting. When the entire team comes together and builds good Acceptance Criteria, quality becomes a natural, and welcome, part of the development process.

Several things then happen. First, the whole team are forced to agree on the details of the requirements before development starts. Before any technical decisions or implementation planning, the exact requirements are clarified. Only by making this an imperative part of the process (usually the refinement meeting) can this happen effectively.

Good Acceptance Criteria Drives Good Development

With clear Acceptance Criteria, development is simplified. Development knows exactly what they need to implement, and when they know this, the process of writing automated tests is no longer abstract and difficult. Test Driven Development is a tricky thing to do, in practice. However, with clear goals, a developer can start with "pseudo tests" - tests which are just code comments - but which outline the business logic before coding begins. These will be a granular list of Prerequisite/Action/Assertions - Unit tests, essentially - outlining the required logic. Then, development becomes a process of implementing that logic.

Breaking the logic down before development begins is a very effective way of ensuring that clean code practices, especially the Single Responsibility Principle, are followed. It becomes natural to keep the logic simple, concise, and separated from other concerns, such as infrastructure. It encourages clean Object Orientated design.

Good Acceptance Criteria Drives Good QA

Also QA know exactly what they will be getting. They know what parts of the job will be automated, so they can design better integration, acceptance, and End to End tests, as well as an effective exploratory strategy.

Without this clarity, QA are left with waiting until a job is done and not knowing what they're going to get - which is a recipe for annoying manual regression testing, and no doubt, bugs.

Agile Demands Quality, Quality Brings Clarity

Agile is about iterating fast. You cannot do this when QA are held back by a requirement to constantly manually regression test, or worse still, you avoid regression testing and hope new development doesn't break anything.

With good Acceptance Criteria baked into the process, everyone has clarity. As a developer, this makes your life so much easier. You know what needs to be developed before you start, which allows you to concentrate on technical implementation details. With clear requirements, the code is cleaner, so development and maintenance is easier. This allows the team to iterate fast, able to add new features quickly.

Early automated testing becomes a breeze, and with this automation in place, QA can focus on exploration. With automation, we are able to develop with confidence, and innovate freely.

Quality no longer slows down the development process, but guides it.

That's why developers should care about quality.

Sunday, April 08, 2018

Being a Senior Developer is more than being a long serving developer

Like many people, I started my development career from another role. I taught myself coding and then built software to help the business and myself. It was well received, but I did not feel like a "real" developer yet.

This experience convinced me that software was what I wanted to do, so I looked for a junior developer role. This wasn't easy, most companies want people who can come in and start being productive right away, and there were many skills I lacked from not working in a "true" software development environment. Eventually, after much persistence, I was taken in as a junior developer for a large finance company.

When handing in my resignation to my non-dev-shop company, I was offered a pay rise to more than I would be getting in the new role at the finance company. I was also offered a new position, the position of "Senior" developer.

Now I had a choice. I could stay, and drive development projects, leading a team of newly hired junior developers. My career would leap to a role of seniority and if I was to move on I could say I had been a "senior" developer.

Or, I could move into a new company as a junior developer, where I would be mentored, and surrounded by more senior developers.

I chose to move.

I wanted to see how it was"really"done. How developers work together, how they feed off each other, how they learn. I wanted to learn. I could not do that if I was the most senior developer.

Staying was never an option. I'd be a fraud. The title of senior can be extremely subjective, given for many reasons. Tenure, age, comparative skill, or genuine skill.

Yet I see this often. Developers who have always been a senior developer don't get to see how it's "really" done, how skilled software departments work together. You can't always work out the best ways to do things by yourself. You need exposure to different practices, methodologies, cultures, personalities, and skill levels.

Moving to a junior role was the best thing I ever did for my career. It taught me to understand the development process from the ground up, filling in many skills gaps from being self taught. Some of these skills were technical, some were interpersonal, but the most important, were both.

Now I realise that the maintainability of our code is one of our most important skills as a developer. It's often said that we write code once and maintain it ten times. Most importantly, other people maintain it. If your code isn't maintainable, then bugs happen.

If your code is not maintainable, it can bring down companies.

Senior developers who have always been senior may not learn the importance of maintainability. They never have the pleasure of being told off by more senior developers for sloppy, non-reusable code. They didn't have the privilege of being humble and being shown how to write code that others can work with. Nobody complains to them about their code because they are always more senior. So they tend to get away with it, and often end up in charge of a hard-to-maintain monolith that nobody wants to work with.

Senior developers should recognise that without exposure to different working dynamics, or other developers who have worked in different environments, they may be missing out.

Management should compliment in-house, home-grown senior developers with senior developers who have experience where they were not the most senior. Well rounded senior developers with a broad range of experience, who are not afraid to speak up when code is not written with others in mind.

If I had stayed as a "senior" developer, I may never have learnt the value of writing code for others.  And I may not even have realised it.

Monday, March 26, 2018

Ways to Spot Single Responsibility Violations

Classes should only have one reason to change. We should attempt to increase cohesion between things that change for the same reason, and decrease the coupling between things that change for different reasons. Classes should do one thing and one thing well.

Violations of the Single Responsibility Principle can cause code to be difficult to test and maintain, and can make it easier for bugs to manifest.

There are a few symptoms of SRP violations which can give warning signs of these issues.

Large classes/methods

Obviously large classes and methods do not always point to SRP violations as there may be a requirement for a significant amount of logic. However, it is usually a good sign.

Large methods are more likely to point to an SRP violation than large classes, or at least too much cyclomatic complexity. You'll become aware of this when you attempt to cover the method with test cases, and realise there are excessive test combinations required.

Too many injected dependencies

While this may depend a lot on how you set up your services, lots of injected dependencies may suggest that the class is doing too much, and may be violating SRP.

Too many tests per class

If a class has too many tests, or the tests need to change often, this is an indication that there may be too much complexity, too much setup, and this may point to an SRP violation.

Described with and/or, or encompassing words such as "manager"

This is a sure sign that your class or method is doing more than one thing. For example: a method called CreateOrEditRecord() or a class called "InvoiceHandler" are red flags, and should be investigated.

Logic in Views

Logic in your front end almost always means that you are violating SRP, because a view's responsibility is to render an output. As soon as you have two pieces of logic in a view, it's unlikely they'll be for the same reason, because they'll be for different elements on the page. This immediately makes it impossible to unit test the view.

Also, because of the standalone nature of views, there's a good chance you'll have to violate DRY in another view.

Move that logic up to a model, a controller, a helper, or a service where it can be properly unit tested.

Thursday, December 17, 2015

Microsoft Certified Solutions Developer!

After 3 years, 3 exams, and an excessive amount of study I have achieved the Microsoft Certified Solutions Developer - Web Applications certification.

I realise that many people don't put a huge amount of emphasis on exams, valuing real world experience instead. But I have found that what I learned studying for these exams has been extremely valuable to my profession, helping me make more informed decisions in my day to day work.

There is no substitute for real world experience, but real world experience leaves a lot of gaps and doesn't always teach the best habits. The exams have enabled me to fill in these gaps and develop a real technical proficiency which I may not have had without them.

Born to Learn just posted this blog talking about a recent study which found 4 advantages certified staff have over non-certified staff, there are some interesting findings! https://borntolearn.mslearn.net/b/weblog/archive/2016/01/25/four-solid-reasons-to-hire-certified-employees