The perils of ignoring compiler warnings

Do not ignore compiler warnings. Simply do not. Not only should you be looking out for compiler errors, you should also be addressing all compiler warnings that are generated when building your solution.

Listed below is a relatively simply method that provides a user source implementation based on the provided name:

	public IUserSource GetUserSource(string name)
        {
            foreach (var userSource in _userSources)
            {
                if (userSource.Alias == name);
                    return userSource;
            }

            return null;
        }

Although simple, this little method contains an subtle typo that caused myself and the team quite some grief. Since the method is simple enough, I did not bother to write any unit test, which also would have brought the problem to fore. Some of you may immediately identify the problem without even attempting to run the code. Others like myself, required an additional set of eyes to scan through each line to find the problem.

The error is on this line:

                if (userSource.Alias == name);

The innocent looking semi-colon at the end of the line short-circuits the entire if condition, effectively always returning the first element in the _userSources list. Therefore, the returned userSource is not neccesarily the one with an Alias which equals to the supplied name.

Interestingly enough, the compiler created the following warning:

Warning	41	Possible mistaken empty statement	C:\dev\src\TestProject\UserSourceProvider.cs	104	47	TestProject

Also, Resharper provided the following additional warning:

"Possibly mistaken empty statement"

which I strangely ignored, since I am usually very particular on ensuring my code is clean and blessed by ReSharper, usually through the green checkbox located at the top right corner of your Visual Studio editor as indicated here clean code indicator.

While I have since corrected the implementation to use a dictionary and added the corresponding unit test, there was a lesson learned here about the perils of ignoring compiler warnings.

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s