5

For the first time I have been coding for an open-source software where all my work gets reviewed before being commited. I understand reviewing work isn't an easy task, so I don't like to waste reviewers' time and effort. But many times I make silly typo errors and errors of other kind while making a change in the code.

For example, the last time when I refactored some old code I moved code from one class to another, but forgot to check that $this only worked in the old class and needed to be changed now.

I do run unit tests, but sometime they miss these errors (we have incomplete test coverage right now). And I agree that many times I am in a hurry to submit the change only to get the work done faster.

But, besides wasting reviewers' time, this actually slows down the development process and reduces my credibility (small errors may even hide praise for good work).

What's the best way to avoid such a scenario or, in other words, how can I review my own code before someone else does?

Edit: I use Notepad++ for PHP, please don't ask me to change the IDE right-away; however, suggestions are welcome if they are persuasive ;)

8
  • 2
    Do you use an IDE?
    – phant0m
    Jul 28, 2012 at 16:31
  • 1
    @nischayn22 , what is you primary environment of work and code editor (IDE)?
    – Yusubov
    Jul 28, 2012 at 16:33
  • Simple: Review your changes before actually committing them.
    – Bernard
    Jul 28, 2012 at 16:46
  • 1
    I used to struggle with this a lot. While automated testing and IDEs are the big winners, I also found breaking problems into smaller pieces helped. It's easier and quicker to find typos in small bits of code than large ones, especially if they slip through your testing and IDE. When I was in my "Write the entire thing at once" phase, catching mistakes took forever.
    – MathAttack
    Jul 29, 2012 at 12:35
  • 3
    Less haste, more speed. Why are you in a hurry?
    – Burhan Ali
    Jul 29, 2012 at 13:54

8 Answers 8

8

You have already answered your question by yourself.

There are several things that one should always remember when developing (no matter what language, IDE, project is on your desk):
- When developing, try to think only about the project you're working on. Everything else should be forgotten for a while, including the praise you'll get if you do your work well. Such thoughts will only distract you, and you'll make more mistakes as result.
- Don't hurry, if it isn't vital for the project. Take your time, try to test everything properly. As you mentioned, the fast-written but full-of-bugs code is not something to be proud of. I'm sure if you don't hurry so much you'll be able to detect many bugs by yourself.

And, as mentioned in other answers, run unit tests. And one more. There will ALWAYS be bugs. No one is perfect. But these two rules will help you reduce their quantity.

9

I do run tests, but sometime they miss these errors (incomplete test coverage).

After your error is discovered, consider adding a new test that would prevent future contributors from repeating this error or better yet, whole kind of errors like this.

If done right, improving test coverage and regression testing will likely sufficiently compensate the damage done by your mistake.

0
5

- Common sense approach would be taking your time and re-visiting your code before marking your work done. In addition, try to maximize your focus/concentration for the peace of work that you are assigned to do.

- First practical approach would be using re-factoring tools like JustCode or similar once. They would improve your productivity and you may tune it up for syntax errors.

- Second practical approach would be using TDD approach. It will make sure that your code has proper tests to function (pass all required unit tests), otherwise it will fail.

Although, TDD takes time to implement and refactor existing code as needed, it is a clean way to write a reliable and solid code.

enter image description here

6
  • 1
    that diagram rarely reflects reality, very often, once you've finished written the test and started writing the implementation that you would discover a bug in the test, that the test is either impossible to pass with the correct implementation or it is testing for the wrong thing altogether.
    – Lie Ryan
    Jul 30, 2012 at 15:52
  • It is just a simple diagram for each code/use case iteration.
    – Yusubov
    Jul 30, 2012 at 15:55
  • It is contrived. Things almost never work that cleanly, you often are forced to work with half-written test and half-written implementation and so you had to go back and forth between writing tests and writing implementation.
    – Lie Ryan
    Jul 30, 2012 at 15:59
  • well, there is no ideal conditions, and changes will happen. That is why with each wave of changes modification to test are included in iteration.
    – Yusubov
    Jul 30, 2012 at 16:08
  • I don't see an arrow going from "Make a little change" to "Add a test"
    – Lie Ryan
    Jul 30, 2012 at 16:09
2

It depends on the programming language. A language with strict static typing and a compilation step in the build process will catch most typos; the stricter the language, the higher the chance of a typo causing invalid syntax. An extreme example is the Haskell programming language - Haskell programmers often claim, half-jokingly, that once a Haskell program compiles, it is usually correct and bug-free. There are still typos that slip through the net though; a famous example is = vs. == in C/C++. Coding style can provide some extra protection; static code analysis may detect suspicious code; a text editor with auto-completion lowers the chance of typos; but ultimately, there is no substitute for actual testing.

If you use a dynamic language, things are more dire - typically, there is no compilation step, so all errors surface as runtime errors, none of them prevent deployment by nature. Worse yet, many errors will only be noticed once the offending code gets executed. This makes (automated) testing even more vital than with strict static compiled languages.

But then, automated tests have their own problem: coverage. For automated tests to catch all errors, you need 100% code coverage (that is, your tests must touch every single line of code), but you also need 100% input coverage (your tests must call each unit with every possible set of inputs, in every possible state of the context). 100% code coverage is doable, but 100% input coverage is hard to impossible for everything non-trivial. Your best bet is to make educated guesses about sets of inputs that might hit edge cases and hope you don't miss any. And since tests are (out of necessity) written by humans, you can make mistakes while writing tests just like you can mistakes while writing the code to be tested. Luckily, more often than not, a bug in the test does not cancel out a bug in the tested code but rather causes the test to fail doubly.

Here's what you can do to prevent and mitigate typos:

  • Be religious about unit-testing everything.
  • Set up your development environment to run all the tests for you automatically: if you have a continuous-integration system in place, configure it to run all unit tests on each build, and consider any build broken if any test fails. Otherwise, consider using source control hooks or similar features to run tests before pushing your changes upstream.
  • Code defensively, avoid 'clever' tricks, use dynamic features conservatively.
  • If your programming language has options to be more strict (e.g. Option Strict and Option Explicit in VisualBasic), turn them on.
  • If your programming language can warn about suspicious code, make it so (not on production systems though), and treat every warning as a fatal error.
  • Make good use of safeguards built into the language (e.g. PHP's type hinting).
  • Use a code editor that provides auto-completion; use it.
  • When naming identifiers, avoid 'near-misses': don't use $User and $user within the same context.
  • Keep your scope as local and small as possible: avoid global state, expose only what you have to, avoid side effects, etc. This avoids accidentally touching things in completely unrelated places, and makes your code easier to test.
  • Have someone else review your code.

None of these are bullet-proof, but combining them covers most of your bases and reduces the risk to, hopefully, acceptable levels.

3
  • Just a thought, but for dynamic languages, you might try to make them as functional as possible -- eliminating the need to worry about code paths and covering all inputs.
    – jmoreno
    Jul 29, 2012 at 1:48
  • @jmoreno: Functional programming is a very powerful idiom, but by itself, it does nothing to reduce the number of code paths to worry about, and its benefit has nothing to do with dynamic vs. static languages. It doesn't solve the typo problem either: if you can mistype a loop variable, you can just as easily mistype a function name or argument. Maybe you mean 'keep things pure' (i.e., 'avoid side effects'), which is sound advice as it makes your functions naturally testable and less prone to obscure bugs. But again, just like functional style, purity does little to counter typos.
    – tdammers
    Jul 29, 2012 at 14:56
  • @jmoreno: added a bullet point in the purity spirit
    – tdammers
    Jul 29, 2012 at 14:58
1

I kept making similar silly mistakes. The solution I found was unit testing. You set up a series of automated tests to check the correctness of your code as you write it.

1

Write your code in a compiled language.

EDIT: I'm not proposing that this is a bullet-proof solution, but it will certainly help you to avoid many typos.

10
  • 3
    It's still very possible to make typos in complied languages. Jul 28, 2012 at 17:03
  • 3
    @TomSquires: Possible, yes, but there is a much higher chance of them being caught before deploying or even committing.
    – tdammers
    Jul 28, 2012 at 21:16
  • Say I work on MediaWiki (software behind Wikipedia), should I ask them to switch from PHP?
    – nischayn22
    Jul 29, 2012 at 7:36
  • 1
    "should I ask them to switch from PHP?" - perhaps not, but you could choose your next project accordingly. It sounds like you're ready to move on from PHP. Jul 29, 2012 at 14:36
  • 1
    +1 … this is actually a big point. Although “compiled language” is slightly inaccurate. The important part is strict checking, as in Perl’s use strict mode. Python, but PHP even more, definitely falls prey to a whole class of typo-related errors which strict checking removes. Jul 30, 2012 at 15:08
0

Find someone who can review your code without embarrassing you.

One of your problems is that you feel you're losing face when the whole project reviews your code and finds errors. Perhaps there's someone either in the project or not who you can pair up with to look at your code. Screenshares, screencasts, patches, work on another branch, etc., any of those methods can be used to share the code before the rest of the project team get to see it.

In return of course, you should offer the same or some other service to that person, e.g., if you double as an artist you could provide icons or whatever they need.

Make sure a past mistake can't happen again

This is easier in a typed language, but if you find that you occasionally write $thsi instead of $this, for instance, enable your editor's spell-checker. If it's something more involved, try to write a test case that catches it.

For instance, a couple of times I made mistakes editing property files used for internationalisation so that there was no English value for some keys. I fixed the files and then wrote a test that loaded up all keys in one go and failed if any were missing.

Invest in better tooling/languages

If there's a better editor than the one you're using, use it. If there's a better language than the one you're using, use it. Of course, 'better' depends on various factors, one of which is the code that you already have in the project. If the target language is really quite bad then you could even generate code from a better language. There are things you lose by doing that but it's good to be aware of that option. In practice I've only had to do that once, when given some requirements for some very repetitive Java code that had to be fully documented. I generated 3000 lines of Java code from 50 lines of another language.

0

A spelling library such as Hunspell or Aspell can be integrated into your program:

library(hunspell)

# Check individual words
words <- c("beer", "wiskey", "wine")
correct <- hunspell_check(words)
print(correct)
# [1]  TRUE FALSE  TRUE

your editor:

notepad++ spellchecking

or your build pipeline:

spell check HTML:
image: tmaier/hunspell:latest
script:
  - export HUNSPELL_FINDINGS=`hunspell -l -i UTF-8 -d de_DE_neu,en_US -p .words -H public/**/*.html | sort | uniq`
  - echo $HUNSPELL_FINDINGS
  - test "$HUNSPELL_FINDINGS" == ""
dependencies:
  - build
stage: test
allow_failure: true

Create a custom dictionary file to spellcheck project specific jargon using the "Add to Dictionary" functionality of your editor of choice.

Add to Dictionary References

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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