CODEWITHSUNDEEP
c#
Danny Beckett
Danny Beckett

Reputation: 20838

Not all code paths return a value - where?

I have the following C# code. For reasons I won't go into, this is the required way of localising.

My problem is, I cannot for the life of me figure out what path is not returning a value. There are no other errors in the below code:

ResourceManager ResMain = new ResourceManager("TorrentSched.Main", typeof(Main).Assembly);
/// <summary>Returns a localised string based on the Current UI Culture</summary>
public string Str(string Name)
{
    Boolean StringNotFound = false;
    switch(Thread.CurrentThread.CurrentUICulture.ToString())
    {
        case "en-GB":
            switch(Name)
            {
                case "MinimizeToTray": return "Closing the program minimises it to the tray. To fully quit the program, right-click the icon in your tray and choose Exit.";
                default: StringNotFound = true; break;
            }
        break;
        default:
            StringNotFound = true;
        break;
    }

    if(StringNotFound)
    {
        StringNotFound = false;
        switch(Name)
        {
            case "AppName":         return ResMain.GetString("$this.Text");
            case "MinimizeToTray":  return "Closing the program minimizes it to the tray. To fully quit the program, right-click the icon in your tray and choose Exit.";
            case "ReallyExit1":     return "Do you really want to exit?";
            case "ReallyExit2":     return "Torrents will not be checked and downloaded until you launch the program again!";
            default:                return "String not found!";
        }
    }
}

Upvotes: 0

Views: 151

Answers (7)

Th3Minstr3l
Th3Minstr3l

Reputation: 322

Maybe you could refactor it so that at the bottom of your code you have a SINGLE return statement. And all your decision-making code simply 'sets' the return value.

Then in your switch(Name) block SET the value of what you want to return - then break out of the switch (rather than having a whole bunch of returns). IMO, this would make the code neater. Also, I think it'd make it easier to maintain.

ie.

switch(Name)
        {
            case "AppName":         retString = ResMain.GetString("$this.Text");
            case "MinimizeToTray":  retString = "Closing the program minimizes it to the tray. To fully quit the program, right-click the icon in your tray and choose Exit.";
            case "ReallyExit1":     retString = "Do you really want to exit?";
            case "ReallyExit2":     retString = "Torrents will not be checked and downloaded until you launch the program again!";
            default:                retString = "String not found!";
        }

...

return retString;

Upvotes: 3

Chadwick13
Chadwick13

Reputation: 357

if(StringNotFound)

If this is false there is no else statment to catch it.

use

if(StringNotFound)
{
//your code
}
Else
{
return "String not found!";
}

Upvotes: 0

Olivier Jacot-Descombes
Olivier Jacot-Descombes

Reputation: 112324

Consider using a dictionary

private static Dictionary<string,string> stringDict = new Dictionary<string,string>();

Add the strings

// Add default strings
stringDict.Add("AppName", ResMain.GetString("$this.Text"));
stringDict.Add("MinimizeToTray", "Closing the program ...");
stringDict.Add("ReallyExit1", "Do you really ...");

// Add culture specific strings
stringDict.Add("en-GB;AppName", ResMain.GetString("$this.Text"));
stringDict.Add("en-GB;MinimizeToTray", "Closing the program ...");
stringDict.Add("en-GB;ReallyExit1", "Do you really ...");

The you can get the strings very quickly with

// Get culture specific string
string culture = Thread.CurrentThread.CurrentUICulture.ToString();
string s;
If (stringDict.TryGetValue(culture + ";" + Name, out s)) {
    return s;
}                          

// Get default
If (stringDict.TryGetValue(Name, out s)) {
    return s;
}

return String.Format("String '{0}' not found!", Name);

This is easier to maintain.

(As other have already pointed out, there is a return-statement missing at the very end of your method and the boolean variable is superfluous.)

Upvotes: 1

Fredrik M&#246;rk
Fredrik M&#246;rk

Reputation: 158309

What is the purpose of the if-block? As far a I can see, if code below the switch is executed, StringNotFound will always be true so the if-block is not necessary, but it may very well confuse the code analysis.

Upvotes: 5

Myrtle
Myrtle

Reputation: 5831

To prevent mistakes or puzzles like this, you better not do both of this (below) , as there is no single logical flow anymore when you:

  • Maintain a retVal variable, together with
  • Multiple uses of the return statement.

Choose one solution:

  • only return a retVal variable, or
  • return a default at the bottom of the function.

Upvotes: 1

Michal Klouda
Michal Klouda

Reputation: 14521

At the very end of your method, if StringNotFound is false:

if(StringNotFound)
{
   [...]
}

// missing return statement here

Upvotes: 3

Honza Brestan
Honza Brestan

Reputation: 10947

If StringNotFound is false, you don't return anything.

Upvotes: 4

Related Questions