CODEWITHSUNDEEP
c#arraystext-files
Tiny
Tiny

Reputation: 419

Reading text file into string array - why am I getting unexpected output?

Trying to create a simple Hangman game whereby an external text (called words.txt) is read and strings therein are imported into an array of string called WordsArray.

Program compiles fine, however, it asks me twice to enter the file name before displaying the contents of the populated array (see foreach loop below)

Can someone identify why it's asking me for filename twice before displaying?

(Also, and more generally, is my refactoring appropriate for this simple application? )

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO;

namespace ConsoleApplication1
{
    class Program
    {
        static string [] LoadWords()
        { 
            bool repeat = true;
            while (repeat)
            {
                Console.WriteLine("Please enter the name of a file:");
                string filename = Console.ReadLine();

                try
                {
                    string[] WordsArray = File.ReadAllLines(filename);
                    if (WordsArray.Length == 0)
                        return null;
                    else
                        return WordsArray;
                }
                catch (FileNotFoundException msg)
                {
                    Console.WriteLine("\n Check the file exists!");
                }
            }
            return null;
        }

        static void DisplayWordsArray(string [] WordsArray)
        {
            foreach (string word in WordsArray)
                Console.WriteLine(word);
        }
        static void Main(string[] args)
        {
            string[] WordsArray= new string[10];
            if (LoadWords() != null)
            {
                Console.WriteLine("File Loaded...\n\n");
                WordsArray = LoadWords();
                DisplayWordsArray(WordsArray);
            }
            Console.ReadLine();
        }
    }
}

Upvotes: 0

Views: 305

Answers (3)

Wilbur Omae
Wilbur Omae

Reputation: 314

As mentioned in some comments and Jesper's answer, the duplicity is due to calling the function LoadWords() twice. It's a common cause of bugs and tends to arise from the need to write as few lines of code as possible (eliminating variable declaration and initialisation by using the function directly).

That aside, it is not prudent to deliberately trigger an exception when the error can be handled otherwise. See the following snippet from the docs:

For conditions that are likely to occur but might trigger an exception, consider handling them in a way that will avoid the exception.

Since a method File.Exists() exists to check the existence of a file, it is proper to use it rather than anticipate an exception.

START_EDIT: since files can be deleted, locked or otherwise changed by external processes, using the try...catch block is prudent as mentioned by @RufusL (check also Eric Lippert's post here). Mea culpa! END_EDIT

Finally, your refactoring is fine: it is almost always best to let a method handle one thing, and handle it well.

Upvotes: 0

ΩmegaMan
ΩmegaMan

Reputation: 31616

Change the logic to only answer the question once, and handle the known failure of missing file possibility. If an exception occurs, exit instead:

static string[] LoadWords()
{
    string[] WordsArray = null; 
    Console.WriteLine("Please enter the name of a file:");

    do
    {
        try
        {  
            string filename = Console.ReadLine();

            if (File.Exists(filename))
                WordsArray = File.ReadAllLines(filename);
            else
                Console.WriteLine($"{Environment.NewLine} File does not exist, try again: ");
        }
        catch (Exception ex)
        {
            Console.WriteLine($"{Environment.NewLine} Unknown exception, exiting. {ex.Message}");
            return null;
        }
    }
     while (WordsArray == null)

    return WordsArray;
}

Upvotes: 0

Jesper Larsen-Ledet
Jesper Larsen-Ledet

Reputation: 6723

It's because you call LoadWords() twice.

You should write:

string[] WordsArray= LoadWords();
if (WordsArray != null)
{
  Console.WriteLine("File Loaded...\n\n");
  DisplayWordsArray(WordsArray);
  ...

Upvotes: 7

Related Questions