CODEWITHSUNDEEP
oopdesign-patterns
samus
samus

Reputation: 6202

Class Methods: Passing private data fields in private methods

I was wondering if which of the following patterns are considered to be more "correct".

The first example sets the value of private data member length by calling a void member function that implicitly takes arguments. The second example sets the value of length by assigning it to the return value of the member function that explicitly takes an argument.

It seems that the second method makes the code clearer, since you know when and how the private member is getting set, where the first requires a trace of the code to verify what and how a value is being assigned. The second method also seems like it would allow for better reuse down the road because it could operate in any class/context (since the arguments and return type are explicit).

The first method is quicker, and if used throughout the entire class (for private member functions) can save some coding, however I'm not sure if this will bite me down the road ?

Thank you for the insight and clearing this up for me.

class MyDataType
{
   private int length;
   private string content;
   private char[] buffer;

   public MyDataType(str) 
   {
      content = str;

      calculateLength();

      buffer = new char[length+1];

      for(int i=0; i < length; i++)
         buffer[i] = content[i];

      buffer[i] = NULL;
   }

   private void calculateLength()
   {
      int i = 0;

      while(content[i++] != NULL) {}  // i know, buffer overflow...

      length = i;
   }
}

class MyDataType
{
   private int length;
   private string content;
   private char[] buffer;

   public MyDataType(str) 
   {
      content = str;

      length = calculateLength(content);

      buffer = new char[length+1];

      for(int i=0; i < length; i++)
         buffer[i] = content[i];

      buffer[i] = NULL;
   }

   private int calculateLength(string s)
   {
      int i = 0;

      while(s[i++] != NULL) {}

      return i;
   }
}

Upvotes: 4

Views: 318

Answers (3)

Tom
Tom

Reputation: 3166

In terms of readability and flexibility the second example wins out.

The advantage to using the second example is that you are essentially just delegating some work in the constructor to a method. This is easy to read and won't easily break in the future if, for instance, someone calls the private calculateLength() method without expecting the instance variables to change.

In the first example the calculateLength() method is operating on the member variables in a non-transparent way to the constructor which makes it less readable and more prone to be brittle in the way described above.

Upvotes: 2

Stephen
Stephen

Reputation: 2424

I would say the second is more appropriate, but the first works.

Usually getters are public, anywhere within a class you can access a private variable, doesn't make sense to have a getter that gets a private or protected variable in the same class. If it were protected and you could pass functionality to subclasses of your class.

It depends on what you want to do with the variable. If the value isn't going to change, I would just move the private calculateLength() method to the constructor. If you have to recalculate it then I guess I can see either the void or private doing pretty much the same thing. I feel that the second is clearer because you know exactly where the calculateLength() method is returning, but in the first it it is ambiguous to whether it is doing anything at all. It's better style to do the second.

Upvotes: 1

d3dave
d3dave

Reputation: 1381

Why not embed the getLength() function content into the constructor, and do the copying simultaneously? You will save yourself another loop. Other than that, I am pretty sure it does not matter much. Just go with the flow, start with a method, and then modify in the future if you see fit.

Upvotes: 0

Related Questions