The size of a Get method

Question

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?

Radical

I am not aware of a specific guideline for properties, but many best-coding practices utilize 7-10 lines as the preferred amount of lines in one method. – Madame

What is your getter doing? – Hodges

Good question. The getter is used in a class that encompasses functionality similar to the following article: umerpasha.wordpress.com/2013/06/13/… You'll notice a lot of stings there (baseString for example) that can be constructed from other members on the class (all the OAuth tokens for example). – Radical

On a related note, the verb choice for your method is often a good hint as to its complexity. See this question on Programmers.SE. – Cubicle

Answer 1

dcastro's answer is good but could use some expansion:

• it doesn't take long to return

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.

• it doesn't connect to external resources (databases, services, etc)

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.

• it doesn't have any side effects

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.

Theism

Thanks for chiming in Eric! I'm not a big fan of quantifying; I think "it shouldn't take much longer than regular data access" is a good prescription. It's not like anyone's gonna actually measure how long it takes. Regarding everything else - spot on! +1 – Andromache

@dcastro: Sure, that's not a hard-and-fast rule but rather a general guideline for how you know when you're too slow. Put another way: you should not have to worry about accessing a property in your inner loop. – Theism

Answer 2

It's not really the size that matters (no pun intended). It's ok to have your logic in a getter as long as

• it doesn't take long to return
• it doesn't connect to external resources (databases, services, etc)
• it doesn't have any side effects

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

Andromache

That was exactly what I was about to write. The "side effects" part is the most important if you ask me. – Odious

+1. Arbitrary line limits are exactly that - arbitrary. We should strive for as small as possible - but splitting methods out just because of an arbitrary limit can impede readability. – Motherinlaw

I would like to add a good read about choosing between properties and methods: msdn.microsoft.com/en-us/library/vstudio/… – Rompers

I'd like to add a simple rule: use a GetMethod when you'd like to explicitly state that it does something more than just "return value" – Dogear

Answer 3

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.

Darsey

Answer 4

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

Soloma

Answer 5

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.

Pepsinogen

I believe methods should be as small as possible and no smaller. Splitting a method out in to other methods just because of some arbitrary limit (one screen - what resolution?) - can impede readability. Common sense prevails above arbitrary limits. – Motherinlaw

whose screen? :) sometimes it's fine to have a long method. Only logic should dictate a method size. – Dogear