Are there any guidelines or general consensus towards the size of a 'Get' in terms of lines of code? I have a Get method on a member that has quite easily grown to 30 lines of code here. I'm not sure at what point this should be pulled out into a method. But then I'd only be calling it something like GetMyString and assigning the value to another member and calling it in the constructor anyway.
Is it ever worth doing this?
Is this too subjective for SO?
dcastro's answer is good but could use some expansion:
That's not quantified; let's quantify that. A property should not take more than, say, ten times longer than it would take to fetch a field.
Those are slow and so typically fall under the first rule, but there is a second aspect to this: failure should be rare or impossible. Property getters should not throw exceptions.
I would clarify that to observable side effects. Property getters often have the side effect that they compute the property once and cache it for later, but that's not an observable side effect.
Not only is it bad philosophically for getting a property to have an observable side effect, it can also mess up your debugging experience. Remember, when you look at an object in the debugger by default the debugger calls its property getters automatically and displays the results. If doing so is slow then that slows down debugging. If doing so can fail then your debugging experience gets full of failure messages. And if doing so has a side effect then debugging your program changes how the program works, which might make it very hard to find the bug. You can of course turn automatic property evaluation off, but it is better to design good properties in the first place.
It's not really the size that matters (no pun intended). It's ok to have your logic in a getter as long as
These are only some of the guidelines for proper property usage.
Edit
The above guidelines share one ideal: Property accessors should behave like data access, because that's what users expect.
From the book Effective C# by Bill Wagner:
Properties are methods that can be viewed from the calling code like data. That puts some expectations into your users’ heads. They will see a property access as though it was a data access. After all, that’s what it looks like. Your property accessors should live up to those expectations. Get accessors should not have observable side effects. Set accessors do modify the state, and users should be able to see those changes.
Property accessors also have performance expectations for your users. A property access looks like a data field access. It should not have performance characteristics that are significantly different than a simple data access.
Property accessors should not perform lengthy computations, or make cross-application calls (such as perform database queries), or do other lengthy operations that would be inconsistent with your users’ expectations for a property accessor.
Bonus by Alberto: http://msdn.microsoft.com/en-us/library/vstudio/ms229054%28v=vs.100%29.aspx
It's not necessarily bad, but if it were me it would make me nervous and I'd be looking to try and break it up somehow. A getter is a method so simply pulling the whole thing into a 30+ line method would be a waste of time in my opinion. I'd be trying to chop it up. E.g. if it was a loop with some checks, extracting the checks as methods or some such.
This is a common bad practice to shove a whole bunch of lines into a Get method. I have something installed in visual studio called CodeMaid. It has something called a CodeMaid Spade which rates each method and gives you a score. The higher the score the worse your method is. It can be used on properties too. I suggest you give it a try, it helps with formatting, indentation and a bunch of other good practices as well
As a general guideline, a method should not have more lines than fit on one screen. If you have to scroll, it's too large. Split it into smaller methods.
