index variable might not be populated if TryGetValue fails, which would caused log.DebugFormat to throw a NullReferenceException ?

Oops, I'm blind too, just realised that "value" is the out parameter :-$

value maybe null if trygetvalue fails. So you have a NullReferenceException when call value.IndexDocuments(...)

Been there, done that! :-)

Ayende, why have you stoped using HTML highlighting?

Not it's only images for code.

OK, try again Ian:

index parameter is null in rare cases, TryGetValue returns false in this case, and log.DebugFormat throws NullReferenceException ?

Um...so, the intention is that when you try to index on a non-existent index it should fail silently? That being the case I'd call the method TryIndex rather than Index and have it return a Boolean to indicate failure.

If a method can't do what its name says it does, it should always throw.

index can be null.

indexes can be null.

value can be null.

Simple.

Even if TryGetValue fails (and value is null), the last statement is still executed. Missing an else.

String.Format fails because 'index' is null.

I think this post was not about "find the bug"! It's about "We all are humans..." ;-)

log is null?

nothing like another pair of eyes at times like that.

What Tommy Carlier said. You forgot 'return' when the TryGetValue fails :-)

I'd be more ashamed of the if (condition == false) bit..

This is serious. It means that even you cannot code 20 hours a day for more than four weeks... ;-)

@Matt

I actually picked up that code style from Ayende. Shorter does not always mean more readable and an exclamation mark is easily overlooked.

I used to apply the exclamation mark, but after trying the "== false" for a while, I'm sold.

Yeah we've all been there, usually a good time for a break.

There should be a "return" statement after log.Debug().

I'm totally with Matt on this one, the (condition == false) is BAD practice. It's SOOOO much easier to overlook than the exclamation mark. The exclamation mark adds the negation beforehand so that you know that you are negating the statement when you read it.

I feel you on the bug though, been there, done that and in the end it's always like DUH!

@Patrick I also use the exclamation mark, but I also think it's not always very obvious (only 1 character that looks like a letter). I'd actually prefer a "not"-keyword here, like in VB.

Maybe the "condition == false" wouldn't be as easy to overlook if you turned it around (like they do in C++ for a different reason):

if (false == condition) { ... }

Actually, now that I think about it: I sometimes create 2 versions of a method: the regular one and the "not"-one. Some extension methods I wrote:

• ICollection <t.HasItems(): same as Count > 0

• ICollection <t.IsEmpty(): same as Count == 0

• string.IsEmpty(): same as string.IsNullOrEmpty(s)

• string.IsNotEmpty(): same as !string.IsNullOrEmpty(s)

Especially the HasItems/IsEmpty on the collection show the intention of the code.

It's not that apparent to me either. I think it's that index will be null if TryGetValue returns false. in which case you either need to return after logging or put value.IndexDocuments in an else block.

@Grimace

false == is even better. Saves time running after stupid bugs.

@Tommy Carlier,

Why not string.HasValue() ? instead of IsNotEmpty()

@Mr_Simple

Sure, its better if you're using a language like C, because if you accidentally use '=' instead of '==' then you'll get a compile error. But C# doesn't have this problem, so I'm not sure what "stupid bugs" you're thinking it prevents.

@Paul Batum

Do it, then come back and let us know what your experience is. I think you'll find it errors out quite nicely thank you.

@Paul Batum

I need to man up Paul.

I owe you a great big apology, once I wipe the egg from my face. You're quite right. I was thinking how assigning with constants on the left upsets the compiler.

I'll crawl back under my rock now with your permission.

I think Oren just need another vacation :)

@alwin I think that "empty" and "not empty" is clearly defined in relation to strings, while "value" is not as clearly defined: what is considered a value?

@Tommy Carlier,

Well of course it's all just a matter of personal taste. For me a string has a value if it has something in it, and yes, spaces are a value too.

I have this:

public static bool IsNullOrEmpty(this string self)

{

return string.IsNullOrEmpty(self);

}

public static bool HasValue(this string self)

{

return !IsNullOrEmpty(self);

}

With HasValue I have one less negation wrt IsNotEmpty, or !IsNullOrEmpty.