Search code examples
c#encapsulationprivate-methods

Whether to use private or public methods


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;
}

Solution

  • 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.