CODEWITHSUNDEEP
c#
Krowi
Krowi

Reputation: 1625

#if DEBUG for exception handling

I have a public method ChangeLanguage that is in a class library that is supposed to be used by other people who have no idea what the source code is but know what it can do.

public string ChangeLanguage(string language) 
{
    #if DEBUG
        // Check if the new language is not null or an empty string.
        language.ThrowArgumentNullExceptionIfNullOrEmpty("language", GetString("0x000000069"));
    #else
        if (string.IsNullOrEmpty(language))
            language = _fallbackLanguage;
    #endif 
}

For me it looked obvious to check if a language was actually passed to the method and if not, throw an error to the developer. Tho this is a very small performance loss, I think it's better to not throw an exception but just use the fallback language when no language was provided.

I'm wondering if this is a good design approach and if I should keep using it at other places in the library like here:

    _appResDicSource = Path.Combine("\\" + _projectName + ";component", _languagesDirectoryName, _fileBaseName + "_" + language + ".xaml");
    _clsLibResDicSource = "\\Noru.Base;component\\Languages\\Language_" + language + ".xaml";

    ResourceDictionary applicationResourceDictionary;
    ResourceDictionary classLibraryResourceDictionary;

    try { applicationResourceDictionary = new ResourceDictionary { Source = new Uri(_appResDicSource, UriKind.RelativeOrAbsolute) }; }
    catch
    {
#if DEBUG
        throw new IOException(string.Format(GetString("1x00000006A"), _appResDicSource));
#else
        return ChangeLanguage(_fallbackLanguage);
#endif
    }
    try { classLibraryResourceDictionary = new ResourceDictionary { Source = new Uri(_clsLibResDicSource, UriKind.RelativeOrAbsolute) }; }
    catch
    {
#if DEBUG
        throw new IOException(string.Format(GetString("1x00000006B"), _clsLibResDicSource));
#else
        return ChangeLanguage(_fallbackLanguage);
#endif
    }

Upvotes: 4

Views: 304

Answers (4)

AlexD
AlexD

Reputation: 32586

It depends on semantics of the call, but I would consider Debug.Fail (which will be eliminated if DEBUG is not defined):

public string ChangeLanguage(string language) 
{
    if (string.IsNullOrEmpty(language))
    {
        Debug.Fail("language is NOK");
        language = _fallbackLanguage;
    }
}

The issue is that, on the one hand, as mentioned by @OndrejTucny and @poke, it is not desirable to have different logic for different build configurations. It is correct.

But on the other hand, there are cases where you do not want the application to crash in the field due to a minor error. But if you just ignore the error unconditionally, you decrease the chances to detect it even on the local system.

I do not think that there is a universal solution. In general, you may end up deciding to throw or not, to log or not, to add an assertion or not, always or sometimes. And the answers depend on a concrete situation.

Upvotes: 5

poke
poke

Reputation: 388223

I think this is a bad idea. Mostly because this separates the logic of a debug build and a release build. This means that when you—as a developer—only build debug builds you will not be able to detect errors with the release-only logic, and you would have to test that separately (and in case of errors, you would have a release build, so you couldn’t debug those errors properly). So this adds a huge testing burden which needs to be handled for example by separate unit tests that run on debug and release builds.

That being said, performance is not really an issue. That extra logic only happens in debug builds and debug builds are not optimized for performance anyway. Furthermore, it’s very unlikely that such checks will cause any noticeable performance problems. And unless you profile your application and actually verify that it is causing performance problems, you shouldn’t try to optimize it.

Upvotes: 1

Ondrej Tucny
Ondrej Tucny

Reputation: 27974

No, this is certainly not a good design approach. The problem is your method's DEBUG and RELEASE contracts are different. I wouldn't be happy using such API as a developer.

The inherent problem of your solution is that you will end up with production behavior that cannot be reproduced in the dev/test environment.

Either the semantics of your library is such that either not providing a language code is an error, then always raise an exception, or it is a valid condition that leads to some pre-defined behavior, such as using the 'fallback' language code, then always substitute the default. However, it shouldn't be both and decided only by the selection of a particular compilation of your assembly.

Upvotes: 4

Jonathon Reinhart
Jonathon Reinhart

Reputation: 137517

How often is this method getting called? Performance is probably not a concern.

I don't think the decision to select the fallback language is the library's to make. Throw the exception, and let the clients choose to select a default, fallback if desired.

Upvotes: 0

Related Questions