Issues with team leader on client site

[Contreras]

99% of comments are unnecessary in this way.

Because you completely failed to describe specifics of the intended behaviour, or provide any explanation or reference to the requirement(s) your code is intending to meet. UnbearablyLongFunctionNames() and equallyUnbearableVariableNames in no way mitigates that.

There is so much bad karma with that snippet that I really don't know where to start.

[kal]

Because you completely failed to describe specifics of the intended behaviour, or provide any explanation or reference to the requirement(s) your code is intending to meet. UnbearablyLongFunctionNames() and equallyUnbearableVariableNames in no way mitigates that.

There is so much bad karma with that snippet that I really don't know where to start.

If you can't work out whats going on from the code then what hope have you of salvaging intent from a few random comments...

[Contreras]

If you can't work out whats going on from the code then what hope have you of salvaging intent from a few random comments...

Ahem. Yes it can be worked out easily enough, no thanks to the UnbearableClutterOfSelfDocumentingCode();

I can only refer the learned gentlemen to my previous rant. We probably agree, at some margin, and I'd rather be enjoying the sunshine.

[d000hg]

(badly written code, constant refactoring because she changes her mind, bad practice, etc).

If you keep refactoring, surely the code quality improves?

I know commented code is bad practice because it encourages poor coding. Code should be self-documenting.

Incorrect

We were behind in our sprint and when I mentioned we need unit tests, she said 'we don't have time for that'. It's only since we installed a code coverage tool (Sonarqube) that the lack of coverage has been highlighted to senior management and not she is hell bent on getting code coverage. Not because she thinks its necessary, but because she's been told by management.

So what - you got what you wanted.

Just do your work and stay out of it.

[tomtomagain]

my team leader is only concerned with her looking good

Well make her look good and all your problems will be over.

[SpontaneousOrder]

Because you completely failed to describe specifics of the intended behaviour, or provide any explanation or reference to the requirement(s) your code is intending to meet. UnbearablyLongFunctionNames() and equallyUnbearableVariableNames in no way mitigates that.

Unit tests
Acceptance tests (BDD?)

Plus such concerns sound more likely to be a problem with SRP violations.

[kal]

Unit tests
Acceptance tests (BDD?)

Plus such concerns sound more likely to be a problem with SRP violations.

+1, Tests should document (in code not comments ) exactly that. What's better a comment saying 'this method should return X if I send in Y' or a test that proves it.

[SpontaneousOrder]

+1, Tests should document (in code not comments ) exactly that. What's better a comment saying 'this method should return X if I send in Y' or a test that proves it.

I like the

@Test
public void UnitBeingTested_Condition_Expectation() {
// do some test stuff
}

Kind of style that Roy Osherove uses in his work. Not to everyone's taste.
i just find some code that is pretty self-describin in it's own right, and then press ctrl-alt-h to bring up a call heirarchy, and click into all of the lovely junit tests which (given the format above) tell me exactly how it works.

At a higher level BDD frameworks like JBehave (for java) can generate lovely human readable, template able, reports from your tests, which can nicely serve as requirements specs in their own right.

[d000hg]

+1, Tests should document (in code not comments ) exactly that. What's better a comment saying 'this method should return X if I send in Y' or a test that proves it.

Unit tests are different. Decent unit test frameworks allow you to bake in a comment to the test so that when the test fails, you get a pretty explanation.

[SpontaneousOrder]

Unit tests are different. Decent unit test frameworks allow you to bake in a comment to the test so that when the test fails, you get a pretty explanation.

I think he's suggesting that you can write good quality tests just you can write good quality production code. I.e. a good test should tell a clear story in and of itself - only asserting on a single thing per test, appropriate use of mocks, etc, etc.

One of the reasons I like the roy osherove approach is that you can generally leave out that baked in comment, which is just another point of potential mistake in your test (think of things like getting the parameters in the wrong order or something so it says 'expected X, got Y' when you really wanted 'Expected Y, got X'), because the expectation (which wasn't met) is part of the test method's name (and your only asserting on one thing, of course).

It's not always practical, but as a general rule I like it.