perf icon indicating copy to clipboard operation
perf copied to clipboard

Confusing code

Open pwaller opened this issue 6 years ago • 1 comments

This is only a minor suggestion, but I found this confusing:

https://github.com/acln0/perf/blob/239c48f711bc4741a1a6e4ab75edf78ca429a7cd/record_test.go#L219-L225

  1. Variables/consts beginning with the name err are usually errors.

  2. It refers to testErrDisabledProcessExist which doesn't exist.

  3. It's unclear from the comment which side of the branch is "in" something. Is it in the returning side, or the non-returning side? This might be a rare case where "early return" is a minor hinderance and perhaps it would be better to phrase it positively, and move the body of code out, to make it clear when the special condition matches:

    func init() {
      if specialCondition {
        doSpecialCondition()
      }
    }
    

pwaller avatar Jul 20 '19 19:07 pwaller

This is indeed quite confusing, and bad. It's probably a relic from before a refactor. All of these self-exec tests need to be checked out more intimately anyway (#12), so I'll clean this up when I get to that. Thanks.

acln0 avatar Jul 20 '19 19:07 acln0