Should I add redundant code now just in case it may be needed in the future?

What you are doing in the you code above is not so much future proofing as much as it is defensive coding.

Both if statements test for different things. Both are appropriate tests depending on your needs.

Section 1 tests for and corrects a null object. Side note: Creating the list doesn't create any child items (e.g., CalendarRow).

Section 2 tests for and corrects user and/or implementation errors. Just because you have a List<CalendarRow> doesn't mean that you have any items in the list. Users and implementors will do things that you can't imagine just because they are allowed to do it, whether it makes sense to you or not.

I guess, this question is basically down to taste. Yes, it is a good idea to write robust code, yet the code in your example is a slight violation of the KISS principle (as a lot of such "future proof" code will be).

I personally would likely not bother making code bullet-proof for the future. I don't know the future, and as such, any such "future bullet-proof" code is doomed to fail miserably when the future arrives anyway.

Instead, I would prefer a different approach: Make the assumptions that you make explicit using the assert() macro or similar facilities. That way, when the future comes around the door, it will tell you precisely where your assumptions do not hold any more.

The definition of 'redundant code' and 'YAGNI' often depend I find on how far you look ahead.

If you experience a problem, then you tend to write future code in such a way as to avoid that problem. Another programmer who hadnt experienced that particular issue might consider your code redundant overkill.

My suggestion is to keep track of how much time you spend on 'stuff that hasnt erred yet' if its loads and you peers are banging out features faster than you, then decrease it.

However, if you are like me, I expect you will have just typed it all out 'by default' and it hasnt really taken you any longer.

Estimate the cost of adding that code now. It will be relatively cheap because it's all fresh in your mind, so you will be able to do this quickly. Adding unit tests is necessary - there's nothing worse then using some method a year later, it doesn't work, and you figure out it was broken from the start and never actually worked.

Estimate the cost of adding that code when it is needed. It will be more expensive, because you have to come back to the code, remember it all, and it is a lot harder.

Estimate the probability that the additional code will actually be needed. Then do the maths.

On the other hand, code full with assumptions "X will never happen" is awful for debugging. If something doesn't work as intended, it means either a stupid mistake, or a wrong assumption. Your "X will never happen" is an assumption, and in the presence of a bug it is suspicious. Which forces the next developer to waste time on it. It's usually better to not rely on any such assumptions.

Two of the ten programming commandments are relevant here:

• Thou shall not assume that input is correct

• Thou shall not make code for future use

Here, checking for null is not "making code for future use". Making code for future use is things like adding interfaces because you think they might be useful "some day". In other words, the commandment is to not add layers of abstraction unless they are needed right now.

Checking for null has nothing to do with future use. It has to do with Commandment #1: do not assume input will be correct. Never assume your function will receive some subset of input. A function should respond in a logical way no matter how bogus and messed up the inputs are.

Another principal you might want to think about is the idea of failing fast. The idea is that when something goes wrong in your program, you want to stop the program altogether immediately, at least while you're developing it before releasing it. Under this principal, you want to write lots of checks to make sure your assumptions hold, but seriously consider having your program stop dead in its tracks whenever the assumptions are violated.

To put it boldly, if there's even a small error in your program, you want it to completely crash while you watch!

This might sound counterintuitive, but it lets you discover bugs as quickly as possible during routine development. If you're writing a piece of code, and you think it's finished, but it crashes when you test it, there's no question that you're not finished yet. Furthermore, most programming languages give you excellent debugging tools that are easiest to use when your program crashes completely instead of trying to do its best after an error. The biggest, most common example is that if you crash your program by throwing an unhandled exception, the exception message will tell you an incredible amount of information about the bug, including which line of code failed and the path through your code the program took on its way to that line of code (the stack trace).

For more thougts, read this short essay: Don't Nail Your Program in the Upright Position

This is relevant to you because it's possible that sometimes, the checks you're writing are there because you want your program to try to keep running even after something's gone wrong. For example, consider this brief implementation of the Fibonacci sequence:

// Calculates the nth Fibonacci number
int fibonacci(int n) {
    int a = 0;
    int b = 1;

    for(int i = 0; i < n; i++) {
        int temp = b;
        b = a + b;
        a = temp;
    }

    return b;
}

This works, but what if someone passes a negative number to your function? It won't work then! So you'll want to add a check to make sure that the function is called with a nonnegative input.

It might be tempting to write the function like this:

// Calculates the nth Fibonacci number
int fibonacci(int n) {
    int a = 0;
    int b = 1;

    // Make sure the input is nonnegative
    if(n < 0) {
        n = 1; // Replace the negative input with an input that will work
    }

    for(int i = 0; i < n; i++) {
        int temp = b;
        b = a + b;
        a = temp;
    }

    return b;
}

However, if you do this, then later accidentally call your Fibonacci function with a negative input, you'll never realize it! Worse, your program will probably keep on running but start producing nonsensical results without giving you any clues as to where something went wrong. These are the hardest types of bugs to fix.

Instead, it's better to write a check like this:

// Calculates the nth Fibonacci number
int fibonacci(int n) {
    int a = 0;
    int b = 1;

    // Make sure the input is nonnegative
    if(n < 0) {
        throw new Exception("Can't have negative inputs to Fibonacci");
    }

    for(int i = 0; i < n; i++) {
        int temp = b;
        b = a + b;
        a = temp;
    }

    return b;
}

Now, if you ever accidentally call the Fibonacci function with a negative input, your program will immediately stop and let you know that something's wrong. Furthermore, by giving you a stack trace, the program will let you know which part of your program tried to execute the Fibonacci function incorrectly, giving you an excellent starting point for debugging what's wrong.

It is a good idea to document any assumptions about parameters. And it is a good idea to check that your client code does not violate those assumptions. I would do this:

/** ...
*   Precondition: rows is null or nonempty
*/
public static CalendarRow AssignAppointmentToRow(Appointment app, List<CalendarRow> rows)
{
    Assert( rows==null || rows.Count > 0 )
    //1. Is rows equal to null? - This will be the case if this is the first appointment.
    if (rows == null) {
        rows = new List<CalendarRow> ();
        rows.Add (new CalendarRow (0));
        rows [0].Appointments.Add (app);
    }

    //blah...
}

[Assuming this is C#, Assert might not be the best tool for the job, as it is not compiled in released code. But that is a debate for another day.]

Why is this better than what you wrote? Your code makes sense if, in a future where your client has changed, when the client passes in an empty list, the right thing to do will be to add a first row and add app to its appointments. But how do you know that that will be the case? It is better to make fewer assumptions now about the future.

As an exercise, first let's verify your logic. Though as we'll see, you have bigger problems than any logical problem.

Call the first condition A and the second condition B.

You first say:

Looking specifically at section two, I know that if section one is true, then section two will also be true.

That is: A implies B, or, in more basic terms (NOT A) OR B

And then:

I can't think of any reason as to why section one would be false and section two evaluate true, which makes the second if statement redundant.

That is: NOT((NOT A) AND B). Apply Demorgan's law to get (NOT B) OR A which is B implies A.

Therefore, if both your statements are true then A implies B and B implies A, which means they must be equal.

Therefore yes, the checks are redundant. You appear to have four code paths through the program but in fact you only have two.

So now the question is: how to write the code? The real question is: what is the stated contract of the method? The question of whether the conditions are redundant is a red herring. The real question is "have I designed a sensible contract, and does my method clearly implement that contract?"

Let's look at the declaration:

public static CalendarRow AssignAppointmentToRow(
    Appointment app,    
    List<CalendarRow> rows)

It's public, so it has to be robust to bad data from arbitrary callers.

It returns a value, so it should be useful for its return value, not for its side effect.

And yet the name of the method is a verb, suggesting that it is useful for its side effect.

The contract of the list parameter is:

• A null list is OK
• A list with one or more elements in it is OK
• A list with no elements in it is wrong and should not be possible.

This contract is insane. Imagine writing the documentation for this! Imagine writing test cases!

My advice: start over. This API has candy machine interface written all over it. (The expression is from an old story about the candy machines at Microsoft, where both the prices and the selections are two-digit numbers, and it is super easy to type in "85" which is the price of item 75, and you get the wrong item.)

Here's how to design a sensible contract:

Make your method useful for either its side effect or its return value, not both.

Do not accept mutable types as inputs, like lists; if you need a sequence of information, take an IEnumerable. Only read the sequence; do not write to a passed-in collection unless it is very clear that this is the contract of the method. By taking IEnumerable you send a message to the caller that you are not going to mutate their collection.

Never accept nulls; a null sequence is an abomination. Require the caller to pass an empty sequence if that makes sense, never ever null.

Crash immediately if the caller violates your contract, to teach them that you mean business, and so that they catch their bugs in testing, not production.

Design the contract first to be as sensible as possible, and then clearly implement the contract. That is the way to future-proof a design.

Now, I've talked only about your specific case, and you asked a general question. So here is some additional general advice:

• If there is a fact that you as a developer can deduce but the compiler cannot, then use an assertion to document that fact. If another developer, like future you or one of your coworkers, violates that assumption then the assertion will tell you.

• Get a test coverage tool. Make sure your tests cover every line of code. If there is uncovered code then either you have a missing test, or you have dead code. Dead code is surprisingly dangerous because usually it is not intended to be dead! The incredibly awful Apple "goto fail" security flaw of a couple years back comes immediately to mind.

• Get a static analysis tool. Heck, get several; every tool has its own particular specialty and none is a superset of the others. Pay attention to when it is telling you that there is unreachable or redundant code. Again, those are likely bugs.

If it sounds like I'm saying: first, design the code well, and second, test the heck out of it to make sure it is correct today, well, that's what I'm saying. Doing those things will make dealing with the future much easier; the hardest part about the future is dealing with all the buggy candy machine code people wrote in the past. Get it right today and the costs will be lower in the future.