Read on Twitter
Another week, another legacy project. Here are some things that I found in the code that you may want to avoid doing in your own projects.
Division by zero gets silenced. The result of the entire operation becomes false, which conveniently gets cast to zero as well.
Lessons:
- Check for a zero before dividing and make your own decision.
- Don't let PHP cast false/string/null to zero. Catch those values early.
Lessons:
- Check for a zero before dividing and make your own decision.
- Don't let PHP cast false/string/null to zero. Catch those values early.
A very large chunk of code inside a try, with the catch also failing because the $result hasn't been assigned a value yet.
- Don't let your catch rely on operations inside the try.
- Ideally, have only one line inside a try block. You can have an outer try/catch for the app.
- Don't let your catch rely on operations inside the try.
- Ideally, have only one line inside a try block. You can have an outer try/catch for the app.
A "unit" test that connects to another server.
- Unit tests shouldn't connect to servers/databases/filesystem.
- End-to-end tests can connect to mocked services. For example, you can use Wiremock in your Docker environment (tutorial: https://medium.com/@jw_ng/mocking-with-wiremock-and-docker-1f1601bd10e4).
- Unit tests shouldn't connect to servers/databases/filesystem.
- End-to-end tests can connect to mocked services. For example, you can use Wiremock in your Docker environment (tutorial: https://medium.com/@jw_ng/mocking-with-wiremock-and-docker-1f1601bd10e4).
Incorrect implementation of the topological sorting algorithm, which may cause incorrect calculations on billions of monetary transactions.
- Use existing packages for generic algorithms when possible. For topological sort, I used this: https://github.com/marcj/topsort.php
- Use existing packages for generic algorithms when possible. For topological sort, I used this: https://github.com/marcj/topsort.php
Someone copy-pasted the actual output of an integration test and put it in the assertion. But that value was wrong due to a bug. Once the bug was fixed, the test started failing.
- Expectations for a test's output should be hand-crafted. Otherwise, you can be endorsing bugs.
- Expectations for a test's output should be hand-crafted. Otherwise, you can be endorsing bugs.
Disregard for initialization:
- Pushing to an undefined array.
- Setting the property of an undefined object.
- Unsetting keys of an undefined array.
- Concatenating undefined variables.
- Multiplying by undefined variables.
- Pushing to an undefined array.
- Setting the property of an undefined object.
- Unsetting keys of an undefined array.
- Concatenating undefined variables.
- Multiplying by undefined variables.
A test failure is dependent on an unknown previous test in the same suite. This means that I have to run it for 27 minutes each time I want feedback.
- Each test much be independent.
- In addition to your E2E tests, also write unit tests to quickly pinpoint issues.
- Each test much be independent.
- In addition to your E2E tests, also write unit tests to quickly pinpoint issues.
An E2E test fails. The expected output is an XML containing 6,602 lines. That is a lot of output and even more lines of code to sift through.
Recommendation:
Create concise expectations, usually by only checking the few relevant tags, while validating the structure via XSD.
Recommendation:
Create concise expectations, usually by only checking the few relevant tags, while validating the structure via XSD.
