I use nested procs for recursive means (FloodFill for example) and utility (AddItemToList kind of thing), but they are usually within 15-25 lines. I especially hate nested procs with a big var section above declaring all the vars used god-knows-where ))
I'm making them all private methods on the class in question. So far, I haven't had to change the parameter list of any of them, which makes me wonder why they were nested in the first place. But I'm repeating myself.....
Nick Hodges I am never sure if I should use nested procedures (which I do sometimes) or extract them to private methods as the latter implies that the extracted procedure might be useful from any other code but mostly isn't and only bloats the interface of the class.
I don't mind a lot of private methods -- better than nested procedures, and the work needs to get done somewhere, preferably in lots of discrete, refactored methods.
You said, you could move the nested procedures out without any change, so they already have been discrete. The difference whether they are nested or private is not so clear to me although I generally would move them to private methods also. But the "nested" approach does not smell to me.
Simon Stuart I fully agree also sometimes it would be justified to extract a nested procedure to a private one. I am not sure about this, both approaches seem to be ok to me.
Simon Stuart Yes, a private methods is exposing more, and when doing that, one needs to make sure what it does is reusable enough, and it wasn't only covering a very specific functionality.
Typical usage case for nested methods can be a simple finite state automaton, where the automaton's scope is limited to the host procedure. Moving those out requires bubbling up the state of the automaton, which either breaks encapsulation or makes thread-safety more precarious. There are also local helpers methods, such as specific error reporting or logging methods.
The main problem, of course, is that the main class in question is a form, and all these methods become methods on a form. And the form is doing 143 things.
Decoupling the business logic from the interface will have to come a little farther down the road.
When in a hurry and the IDE version is the buggy one (class navigation simply do not work) i feel i can save time using nested functions. But i try not to declare common variables as that can prove a pitfall.
I use to be anti-nested procedures but that was too black and white. I will use a nested proc when I have a set of operations that are repeated several times within a single method. Another guideline I loosely follow is that the nested proc must only be a few lines long otherwise it will get promoted to a private method.
What I will not stand for is nested procedures within nested procedures - at any level of depth.
My problem with nested methods is ambiguity. Did you mean the i variable declared in the nested procedure or the one in the host? Oh you never make that mistake. But someone fixing your code a year or 2 down the line might. Make it private and the ambiguity goes. My first rule is always avoid the potential for ambiguity.
Simon Stuart you're overriding my right to public comment, and more importantly have those comments published. I assume I am protected, perhaps strictly so, in this, by the laws of the land. :-)
Steven Camilleri don't define procedure "global" vars above your nested procedure and this won't happen. If you need a var in a nested procedure, pass it over. Apart from that, I'm following exactly the same approach as Simon Stuart and I'm running fine with it.
Would be interesting to hear the outcome - how did you refactored it.
ReplyDeleteI never liked nested procedures. I always design so that there is none.
ReplyDeleteI use nested procs for recursive means (FloodFill for example) and utility (AddItemToList kind of thing), but they are usually within 15-25 lines. I especially hate nested procs with a big var section above declaring all the vars used god-knows-where ))
ReplyDeleteSimon Stuart I would guess someone mistakenly deleted a line in the first place! ;-)
ReplyDeleteI'm making them all private methods on the class in question. So far, I haven't had to change the parameter list of any of them, which makes me wonder why they were nested in the first place. But I'm repeating myself.....
ReplyDeleteNick Hodges
ReplyDeleteI am never sure if I should use nested procedures (which I do sometimes) or extract them to private methods as the latter implies that the extracted procedure might be useful from any other code but mostly isn't and only bloats the interface of the class.
I don't mind a lot of private methods -- better than nested procedures, and the work needs to get done somewhere, preferably in lots of discrete, refactored methods.
ReplyDeleteYou said, you could move the nested procedures out without any change, so they already have been discrete. The difference whether they are nested or private is not so clear to me although I generally would move them to private methods also. But the "nested" approach does not smell to me.
ReplyDeleteSimon Stuart
ReplyDeleteI fully agree also sometimes it would be justified to extract a nested procedure to a private one. I am not sure about this, both approaches seem to be ok to me.
If it is not written in uppercase (aka BEGIN ... END) and flushed left without any indent then you haven't seen the worst of it ;)
ReplyDeleteSimon Stuart Yes, a private methods is exposing more, and when doing that, one needs to make sure what it does is reusable enough, and it wasn't only covering a very specific functionality.
ReplyDeleteTypical usage case for nested methods can be a simple finite state automaton, where the automaton's scope is limited to the host procedure. Moving those out requires bubbling up the state of the automaton, which either breaks encapsulation or makes thread-safety more precarious.
There are also local helpers methods, such as specific error reporting or logging methods.
The main problem, of course, is that the main class in question is a form, and all these methods become methods on a form. And the form is doing 143 things.
ReplyDeleteDecoupling the business logic from the interface will have to come a little farther down the road.
When in a hurry and the IDE version is the buggy one (class navigation simply do not work) i feel i can save time using nested functions. But i try not to declare common variables as that can prove a pitfall.
ReplyDeleteI use to be anti-nested procedures but that was too black and white. I will use a nested proc when I have a set of operations that are repeated several times within a single method. Another guideline I loosely follow is that the nested proc must only be a few lines long otherwise it will get promoted to a private method.
ReplyDeleteWhat I will not stand for is nested procedures within nested procedures - at any level of depth.
My problem with nested methods is ambiguity. Did you mean the i variable declared in the nested procedure or the one in the host? Oh you never make that mistake. But someone fixing your code a year or 2 down the line might. Make it private and the ambiguity goes. My first rule is always avoid the potential for ambiguity.
ReplyDeleteI should move it to a strict private conversation then Simon Stuart :-)
ReplyDeleteSimon Stuart You are sometimes ambiguous but brilliant :P
ReplyDeleteSimon Stuart you're overriding my right to public comment, and more importantly have those comments published. I assume I am protected, perhaps strictly so, in this, by the laws of the land. :-)
ReplyDeletewow i'm witnessing an object oriented arguments :P I really hope it doesn't lead to procedural trashing... :)
ReplyDeleteSteven Camilleri don't define procedure "global" vars above your nested procedure and this won't happen. If you need a var in a nested procedure, pass it over.
ReplyDeleteApart from that, I'm following exactly the same approach as Simon Stuart and I'm running fine with it.
Lübbe Onken true. We just made up two rules. Both work, I guess it ultimately comes down to preference.
ReplyDeleteI don't mind nested methods. I try not to have them below the var section of a procedure.
ReplyDelete