Whether to use private or public methods [closed]

Question

Closed. This question needs details or clarity. It is not currently accepting answers.

---

Want to improve this question? Add details and clarify the problem by editing this post.

Closed 8 years ago.

Improve this question

As I continue to further enhance my hangman game in C# to help me learn the language and think like a programmer, there are methods in it that, I think, should be in separate classes. Right now, all the code is on the Form class (Windows Form). This makes calling the needed methods really easy as I only have to use method name and use the necessary parameters. And, as it's only a simple hangman game, this may be the best way to do it. I don't know.

Specifically, one of the methods that I have reads a .txt file that has thousands of words in it, splits it into an array, shuffles the array, then calls another method to do something else.

As I read through some of the literature on C#, I'm told that you want to hide as much of your class as possible. That way you can't break your class by passing data that it can't handle. But this seems to mean that I must add a property to that class in order to access it, in addition to having to create an object of that class, just to be able to use the method I want. That seems like sort of a Byzantine way to just get access to that method.

As experienced programmers can see, I'm not thinking like a programmer yet. My focus is to get the right habits formed early, rather than have to undo bad habits later.

So the question basically is should this method be set as private in a simple program like this? What's the best practice?

Code in question is the following (method that reads the file, forms array, shuffles. etc):

private void ReadFile(StringBuilder hintlength, string[] wordlist, string lettercountpath) 
{
  string fileContent = File.ReadAllText(lettercountpath); //Read file
  string[] array = fileContent.Split((string[]null, StringSplitOptions.RemoveEmptyEntries); //Form array

  Random rand = new Random(); 

  for (int i = 0; i < array.Length; i++) // Shuffle algorithm 
  {
    int randIndex = rand.Next(i, array.Lenth);
    string temp = array[randIndex];
    array[randIndex] = array[i];
    array[i] = temp;
  }

  for (int i = 0; i < 10; i++0)  //Assigns shuffled array into wordlist array
    wordlist[] = array[i];

  if (j > 9) //Checks counter to see how many times it's been clicked
     j =0;

  Start.Enabled = false;
  NewWord.Enabled = false;

  WordSelection(hint length, wordlist); // Calls WordSelection method 

  radioButton1.Enabled = false;
  radioButton2.Enabled = false;
  radiobutton3.Enabled = false;

  if (remainderWords == 1) // Checks remaining words counter
     remainderWords = 10;
}

Answer 1

You should set visibility of the class members according to their place in the design, not according to the size of the class or any other considerations.

• If a method or a field represents or does something that is relevant to the way the class does its work, but not to the way the users of the class see what it does, mark the member private. The fancy name for this is "implementation details": you do not want to have them exposed to ensure that you can change them later on.
• If a method or a field is essential to what the class does for its users, make that member public: otherwise, nobody would be able to use that member, rendering the entire class useless.
• If your class is designed for inheritance, and a method or a field is prepared for exclusive use by this class and its subclasses, make that method protected.
• If a method or a field is an implementation detail that needs to be visible by other classes inside the same assembly, make the member internal. You can mix internal and protected, further restricting the access to derived classes inside the same assembly.

You should start doing this classification in your mind as you design your software. It is much easier to do this classification when you design smaller systems. However, the importance of doing it right grows tremendously as the size of your system goes up.

Answer 2

There is the concept of having a class for every responsibility. For you Hangman program you need a random word, which you are reading from a file. This is a good time to build a new class: One that reads a word file and gives you a random word.

The part that you will keep private is the actual collection of words. The main game has no benefit by knowing the whole collection, it just needs a random word. Thus, you could build something like:

public class HangmanWordProvider
{
    private string[] _words;

    public HangmanWordProvider(string inputfile) {
        // code to read file into _words variable here
    }

    public string GetRandomWord()
    {
        // code to return a random word from the collection
    }
}

You would then create a new instance of the word provider to use during the game. Your main HangmanGame now no longer needs to bother with reading a word file, or getting a random word from the collection. You just call your wordprovider.GetRandomWord() and know you get the required data. This is separation of concern.

Now imagine your game grows, you want to make sure the provider does not return the same word twice in a row. This would be something you build into the WordProvider, without having to touch the game class itself.

You can go further and at some point use a database or a webservice to provide words... you would still only have to change the WordProvider, not your game.

The private parts in a class are about hiding the implementation of parts that other classes do not need to know about. In your case, the game does not need to know about how the word list is stored, where it is loaded from, or what way you use to get a random result. It only needs to know how it can get a single random word.