Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Say I have a method like this:

public void OrderNewWidget(Widget widget)
{
   if ((widget.PartNumber > 0) && (widget.PartAvailable))
   {
        WigdetOrderingService.OrderNewWidgetAsync(widget.PartNumber);
   }
}

I have several such methods in my code (the front half to an async Web Service call).

I am debating if it is useful to get them covered with unit tests. Yes there is logic here, but it is only guard logic. (Meaning I make sure I have the stuff I need before I allow the web service call to happen.)

Part of me says "sure you can unit test them, but it is not worth the time" (I am on a project that is already behind schedule).

But the other side of me says, if you don't unit test them, and someone changes the Guards, then there could be problems.

But the first part of me says back, if someone changes the guards, then you are just making more work for them (because now they have to change the guards and the unit tests for the guards).

For example, if my service assumes responsibility to check for Widget availability then I may not want that guard any more. If it is under unit test, I have to change two places now.

I see pros and cons in both ways. So I thought I would ask what others have done.

share|improve this question
14  
You are not making "more work" for the maintainers. If they change logic, they must change the corresponding unit tests. That's how it works. I don't see your cons: if your unit test didn't require changing, then it wouldn't be testing anything, would it? You might as well question whether unit testing is useful at all. –  Andres F. Nov 28 '12 at 17:30
9  
This is off topic, but I would change the logic to throw an exception if the part number is 0 or less, or if the part is not available, as in my opinion it would be a bug to allow someone to call that method with a bogus widget, silently masking another problem. –  Matthew Nov 28 '12 at 19:03
2  
@Matthew very good point. This function lies. The naming tells you it's gonna order something. And then it doesn't but you will never know, unless you apply the same logic as is inside, which iviolates DRY. In other words: if the design is changed to be more correct, this question maybe wouldn't have been asked in the first place. –  stijn Nov 28 '12 at 21:20
1  
but it is not worth the time" (I am on a project that is already behind schedule). We are software developers. The only time we are on schedule is when we are dead :) –  maple_shaft Nov 30 '12 at 14:38
add comment

4 Answers 4

up vote 22 down vote accepted

Part of me says "sure you can unit test them, but it is not worth the time" (I am on a project that is already behind schedule).

It's three very short tests. You spent as much time asking yourself the question.

But the other side of me says, if you don't unit test them, and someone changes the Guards, then there could be problems.

Listen to this side.

But the first part of me says back, if someone changes the guards, then you are just making more work for them (because now they have to change the guards and the unit tests for the guards).

If your maintainer is a TDD nut, you're making it more difficult for them. Any change I make without there being a related change or addition of tests leads to my having to think hard. In fact, I would probably add the tests before I go ahead and make the change.

The first part of you is just plain wrong. Give the second part a pat on the back and stop thinking about it.

share|improve this answer
    
+1 for succinctness even though I wrote an answer too heh –  Jimmy Hoffa Nov 28 '12 at 17:51
2  
+1. Yes, if the requirements change, the some tests will have to be adapted. –  Olivier Jacot-Descombes Nov 28 '12 at 18:10
    
While I like what you say, I have many instances of this kind of call in my code (the front half of an async call). So it is not just 3 unit tests. Still, if it is the "right" way then I want to get them done. –  Vaccano Nov 28 '12 at 21:47
    
@Vaccano: The more of them you have to write, the more logical paths you don't have tests for and the more necessary it is to write them. –  pdr Nov 28 '12 at 22:14
add comment

If you don't have time in your schedule for unit testing but you have time put aside for solid QA usage, then ask if you can steal some of that QA time to write unit tests, or if you can spend some of the QA period doing unit tests, or perhaps just deal with non-unit tested code.. Unfortunately schedules that are immovable force you to make concessions or work yourself to death, I generally suggest the first option because the second one will result in you incapable of supporting/maintaining the system correctly for the term of it.

That said, to your general question of testing guard statements; Yes! Absolutely test guard statements! Those are important parts of the behaviour of that method, you wouldn't want to find out somebody misunderstood something doing a bug-fix and removed your guards or changed the && to an || would you? Unit tests will ensure that a) you actually got the logic on your guards correct and b) nobody breaks that logic later without getting a complaint when they run the unit tests telling them it should be that way for some reason.

share|improve this answer
add comment

It would simplify unit testing if the guard logic and the actual ordering were separate methods.

In the Widget class

public bool IsReadyForOrdering { get { return PartNumber > 0 && PartAvailable; } }

or an equivalent method somewhere else

public bool IsWidgetReadyForOrdering(Widget widget)
{
    return widget.PartNumber > 0 && widget.PartAvailable;
}

The order method

public void OrderNewWidget(Widget widget)
{
   if (IsWidgetReadyForOrdering(widget)) {
        WigdetOrderingService.OrderNewWidgetAsync(widget.PartNumber);
   }
}

Now testing IsWidgetReadyForOrdering became easy. Don't think a long time about it any more. Test it!

share|improve this answer
1  
Oof, stateful logic in a property.. -1 that should be a method and it should take the PartNumber, PartAvailable due to it's simplicity. Make it protected if it need not be a part of the public API but not a property.. Also I generally disagree. The guard logic inside the method is totally fine, and in my eyes better because it's such a small method you're just polluting the class to make an already small method smaller. Add it to the class in this case only if it's repetitive logic. –  Jimmy Hoffa Nov 28 '12 at 17:40
1  
First, this doesn't answer the question. Second, it's untrue. You need to write three tests either way. Third, it exposes implementation details to calling code, unnecessarily. –  pdr Nov 28 '12 at 17:50
1  
It's clever, but I agree with the other commenters; there's no value-added here, unless IsReadyForOrdering is used outside of the class. –  Robert Harvey Nov 28 '12 at 17:52
7  
@JimmyHoffa: where do you see any stateful logic in that property? In fact, the example shows how to separate the state-changing logic (OrderNewWidget) from the non-statechanging logic (ReadyForOrdering). And I am a strong believer that extracting even that small functions will improve the code, and not making it worse, as you state. So +1. –  Doc Brown Nov 28 '12 at 17:54
2  
@JimmyHoffa: actually, your definition of "stateless" means "static" in C#. Thus, what you say is "properties should not access state of an object". But even dumb getters access state variables, so that would mean "write only static properties" - which does not seem to make much sense. –  Doc Brown Nov 28 '12 at 20:37
show 11 more comments

if you were feeling really smart, you'd make the widget part number a class that can NEVER BE ZERO OR LESS so you wouldn't need to guard against that.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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