tags:

views:

406

answers:

9

I have a class 'Data' that uses a getter to access some array. If the array is null, then I want Data to access the file, fill up the array, and then return the specific value.

Now here's my question:

When creating getters and setters should you also use those same accessor properties as your way of accessing that array (in this case)? Or should you just access the array directly?

The problem I am having using the accessors from within the class is that I get infinite loops as the calling class looks for some info in Data.array, the getter finds the array null so goes to get it from the file, and that function ends up calling the getter again from within Data, array is once again null, and we're stuck in an infinite loop.

EDIT:

So is there no official stance on this? I see the wisdom in not using Accessors with file access in them, but some of you are saying to always use accessors from within a class, and others are saying to never use accessors from with the class............................................

+4  A: 

I think its a good idea to always use the accessors. Then if you need any special logic when getting or setting the property, you know that everything is performing that logic.

Can you post the getter and setter for one of these properties? Maybe we can help debug it.

Jason Punyon
So then I should find away to set the getters and setters such that they never cause infinite loops...
Alex Baranosky
Right - It's hard to imagine why a function that would retrieve the data should call a getter - a setter sure.
Peter LaComb Jr.
The imagination can be highly limited at times. --> It does.
Alex Baranosky
+1  A: 

If using an Get method leads to this kind of error, you should access the value directly. Otherwise, it is good practice to use your accessors. If you should modify either the getter or setter to take specific actions in the future, you'll break your object if you fail to use that path.

Mark Brittingham
I understand that, hence the confusion on my part. I guess I will just have to take the Load(file) out of the getters, and make the caller call Load manually before accessing.
Alex Baranosky
+1  A: 

You should always use the accessors, but the function that reads the value from the file (which should be private, and called something like getValueFromFile) should only be called when the value has to be read from the file, and should just read the file and return the value(s). That function might even be better off in another class, dedicated to reading values from your data file.

gkrogers
+1  A: 

I guess what you are trying to implement is some sort of a lazy-loading property, where you load the data only when it is accessed for the first time.

In such a case I would use the following approach to prevent the infinite loop:

private MyData _data = null;

public MyData Data
{
  get
  {
    if (_data == null)
      _data = LoadDataFromFile();
    return _data;
  }
}

private MyData LoadDataFromFile()
{
  // ...
}

In other words:

  • don't implement a setter
  • always use the property to access the data (never use the field directly)
M4N
That's pretty much exactly what I have except Load() is looping on itself. I am sure I can write it in a way that will eliminate that problem.
Alex Baranosky
+2  A: 

No. I don't believe you should, the reason: maintainable code.

I've seen people use properties within the defining class and at first all looks well. Then someone else comes along and adds features to the properties, then someone else comes along and tries to change the class, they don't fully understand the class and all hell breaks loose.

It shouldn't because maintenance teams should fully understand what they are trying to change but they are often looking at a different problem or error and the encapsulated property often escapes them. I've see this a lot and so never use properties internally.

They can also be a performance hog, what should be a simple lookup can turn nasty if someone puts database code in the properties - and I have seen people do that too!

The KISS principle is still valid after all these years...!

Fortyrunner
dang. Good point. Now I am unsure which option to choose.
Alex Baranosky
+4  A: 

I have written a getter that opens a file and always regretted it later. Nowdays I would never solve that problem by lazy-constructing through the getter - period. There's the issue of getters with side-effects where people don't expect all kinds of crazy activity to be going on behind the getter. Furthermore you probably have to ensure thread safety, which can further pollute this code. Unit-Testing can also become slightly harder each time you do this.

Explicit construction is a much better solution than all sorts of lazy-init getters. It may be because I'm using DI frameworks that give me all of this as part of the standard usage patterns. I really try to treat construction logic as distinctly as possible and not hide too much, it makes code easier to understand.

krosenvold
so instead I could do something like this from the calling code: if (class.isNull()) then class.Load() , class.[index]
Alex Baranosky
A: 

If I am understanding it right, you are trying to access a property from within it's implementation (by using a method that calls the same property in the property's implementation code). I am not sure if there any official standards regarding this, but I would consider it a bad practice, unless there would be a specific need to do it.

I always prefer using private members within a class instead of properties, unless I need the functionality property implementation provides.

moose-in-the-jungle
+6  A: 

I agree with krosenvold, and want to generalize his advice a bit:

Do not use Property getters and setters for expensive operations, like reading a file or accessing the network. Use explicit function calls for the expensive operations.

Generally, users of the class will not expect that a simple property retrieval or assignment may take a lot of time.

This is also recommended in Microsoft's Framework Design Guidelines.;

Do use a method, rather than a property, in the following situations.

The operation is orders of magnitude slower than a field set would be. If you are even considering providing an asynchronous version of an operation to avoid blocking the thread, it is very likely that the operation is too expensive to be a property. In particular, operations that access the network or the file system (other than once for initialization) should most likely be methods, not properties.

oefe
+2  A: 

Aside from the point made by others, whether to use an accessor or a field directly may need to be informed by semantics. Some times the semantics of an external consumer accessing a property is different from the mechanical necessity of accessing its value by internal code.

Eric Lippert recently blogged on this subject in a couple of posts:-

automatic-vs-explicit-properties
future-proofing-a-design

AnthonyWJones