Home |
Search |
Today's Posts |
#1
Posted to microsoft.public.excel.programming
|
|||
|
|||
A coding style question
Hello,
taking the cue from the previous thread I opened ("Error handling in VBA"), I would like to ask you if in your opinion it's usually better to check input data to a subroutine, inside the subroutine, or to move each check to a distinct subroutine. As an example, I'll post again my Interp subroutine, this time with some modifications added by Peter T: Public Function Interp2(xArr() As Double, yArr() As Double, _ x As Double) As Double Dim i As Long ' this is the part ... If ((x < xArr(LBound(xArr))) Or (x xArr(UBound(xArr)))) Then 'MsgBox "Interp2: x is out of bound" & "X = " & x '.... I am considering moving outside the subroutine Else For i = LBound(xArr) To UBound(xArr) If xArr(i) = x Then Interp2 = yArr(i) Exit For ElseIf xArr(i) x Then Interp2 = yArr(i - 1) + (x - xArr(i - 1)) / _ (xArr(i) - xArr(i - 1)) * (yArr(i) - yArr(i - 1)) Exit For End If Next i End If End Function From one point of view, I would say yes, because a single subroutine should just do a single thing, as someone on this ng recently reminded me (don't remember who). On the other hand, if one follows this principle too strictly, wouldn't he end writing "unsafe" code? If I take out the check from Interp2 and move it to another subroutine which the caller has to call before Interp2, I risk forgetting to add the call to the checker subroutine when I reuse Interp2 in another code. Sure, I could place the call to the checking sub inside Interp2: however, this way I end up having a code with the same functionalities as before, but with the added slowdown due to the call to the checking subroutine. Best Regards Sergio |
#2
Posted to microsoft.public.excel.programming
|
|||
|
|||
A coding style question
If you mean the test in your If..Then statement... no, I would *not* move it
to a separate subroutine. Personally, I think the the principal of one action per subroutine kind of overdoes it sometimes, but your question would not really fall under this principal as I understand it. -- Rick (MVP - Excel) "deltaquattro" wrote in message ... Hello, taking the cue from the previous thread I opened ("Error handling in VBA"), I would like to ask you if in your opinion it's usually better to check input data to a subroutine, inside the subroutine, or to move each check to a distinct subroutine. As an example, I'll post again my Interp subroutine, this time with some modifications added by Peter T: Public Function Interp2(xArr() As Double, yArr() As Double, _ x As Double) As Double Dim i As Long ' this is the part ... If ((x < xArr(LBound(xArr))) Or (x xArr(UBound(xArr)))) Then 'MsgBox "Interp2: x is out of bound" & "X = " & x '.... I am considering moving outside the subroutine Else For i = LBound(xArr) To UBound(xArr) If xArr(i) = x Then Interp2 = yArr(i) Exit For ElseIf xArr(i) x Then Interp2 = yArr(i - 1) + (x - xArr(i - 1)) / _ (xArr(i) - xArr(i - 1)) * (yArr(i) - yArr(i - 1)) Exit For End If Next i End If End Function From one point of view, I would say yes, because a single subroutine should just do a single thing, as someone on this ng recently reminded me (don't remember who). On the other hand, if one follows this principle too strictly, wouldn't he end writing "unsafe" code? If I take out the check from Interp2 and move it to another subroutine which the caller has to call before Interp2, I risk forgetting to add the call to the checker subroutine when I reuse Interp2 in another code. Sure, I could place the call to the checking sub inside Interp2: however, this way I end up having a code with the same functionalities as before, but with the added slowdown due to the call to the checking subroutine. Best Regards Sergio |
#3
Posted to microsoft.public.excel.programming
|
|||
|
|||
A coding style question
Broadly speaking, the validation of input data to a procedure should
be within the procedure itself, and should execute before any other code. However, it might be the case that several different procedures have the same validation needs. For example, you may need to validate whether a server share is available, and do this validation in a number of independent procedures. In this case, you would probably want to move the actual data validation to its own proc and call that proc whenever the validation is required. If multiple procedures need the same validation and that validation is complex, then you should put it in its own proc that returns a Boolean indicating whether the data is valid. There is no benefit, and there is some (very small) cost, to moving data validation code out of the proc that needs it to another proc. Taking it out to another proc just for the sake of taking it out buys you nothing. Whoever told you that a single proc should do a single thing was correct and you would do well to follow that recommendation. However, in this context, I would consider any input validation to be a subordinate task of the primary task carried out by the proc. Cordially, Chip Pearson Microsoft MVP 1998 - 2010 Pearson Software Consulting, LLC www.cpearson.com [email on web site] On Thu, 4 Feb 2010 09:59:34 -0800 (PST), deltaquattro wrote: Hello, taking the cue from the previous thread I opened ("Error handling in VBA"), I would like to ask you if in your opinion it's usually better to check input data to a subroutine, inside the subroutine, or to move each check to a distinct subroutine. As an example, I'll post again my Interp subroutine, this time with some modifications added by Peter T: Public Function Interp2(xArr() As Double, yArr() As Double, _ x As Double) As Double Dim i As Long ' this is the part ... If ((x < xArr(LBound(xArr))) Or (x xArr(UBound(xArr)))) Then 'MsgBox "Interp2: x is out of bound" & "X = " & x '.... I am considering moving outside the subroutine Else For i = LBound(xArr) To UBound(xArr) If xArr(i) = x Then Interp2 = yArr(i) Exit For ElseIf xArr(i) x Then Interp2 = yArr(i - 1) + (x - xArr(i - 1)) / _ (xArr(i) - xArr(i - 1)) * (yArr(i) - yArr(i - 1)) Exit For End If Next i End If End Function From one point of view, I would say yes, because a single subroutine should just do a single thing, as someone on this ng recently reminded me (don't remember who). On the other hand, if one follows this principle too strictly, wouldn't he end writing "unsafe" code? If I take out the check from Interp2 and move it to another subroutine which the caller has to call before Interp2, I risk forgetting to add the call to the checker subroutine when I reuse Interp2 in another code. Sure, I could place the call to the checking sub inside Interp2: however, this way I end up having a code with the same functionalities as before, but with the added slowdown due to the call to the checking subroutine. Best Regards Sergio |
#4
Posted to microsoft.public.excel.programming
|
|||
|
|||
A coding style question
Hi Chip!
Thanks for the reply, and compliments for your website, it's very good and full of interesting information (actually, it was the first site I used to start learning VBA for Excel). If I understood you correctly, you're saying that input data checking is a subtask of the sub itself, so I would do well to leave the check in Interp2. If the check was more complex, and used by many subroutines, I should consider moving it to a separate sub CheckData, and have the various subroutines such as Interp2 call CheckData, so I'm sure that each time Interp2 is called, CheckData is called too. Very well, I'll keep that suggestion in mind. Thank you for your help, Best Regards Andrea On 4 Feb, 19:25, Chip Pearson wrote: Broadly speaking, the validation of input data to a procedure should be within the procedure itself, and should execute before any other code. However, it might be the case that several different procedures have the same validation needs. For example, you may need to validate whether a server share is available, and do this validation in a number of independent procedures. In this case, you would probably want to move the actual data validation to its own proc and call that proc whenever the validation is required. *If multiple procedures need the same validation and that validation is complex, then you should put it in its own proc that returns a Boolean indicating whether the data is valid. There is no benefit, and there is some (very small) cost, to moving data validation code out of the proc that needs it to another proc. Taking it out to another proc just for the sake of taking it out buys you nothing. Whoever told you that a single proc should do a single thing was correct and you would do well to follow that recommendation. However, in this context, I would consider any input validation to be a subordinate task of the primary task carried out by the proc. Cordially, Chip Pearson Microsoft MVP 1998 - 2010 Pearson Software Consulting, LLCwww.cpearson.com [email on web site] On Thu, 4 Feb 2010 09:59:34 -0800 (PST), deltaquattro wrote: Hello, taking the cue from the previous thread I opened ("Error handling in VBA"), I would like to ask you if in your opinion it's usually better to check input data to a subroutine, inside the subroutine, or to move each check to a distinct subroutine. As an example, I'll post again my Interp subroutine, this time with some modifications added by Peter T: Public Function Interp2(xArr() As Double, yArr() As Double, _ * * * * * * * * * * * * * *x As Double) As Double Dim i As Long ' this is the part ... * *If ((x < xArr(LBound(xArr))) Or (x xArr(UBound(xArr)))) Then * * * *'MsgBox "Interp2: x is out of bound" & "X = " & x '.... I am considering moving outside the subroutine * *Else * * * *For i = LBound(xArr) To UBound(xArr) * * * * * *If xArr(i) = x Then * * * * * * * *Interp2 = yArr(i) * * * * * * * *Exit For * * * * * *ElseIf xArr(i) x Then * * * * * * * *Interp2 = yArr(i - 1) + (x - xArr(i - 1)) / _ * * * * * * * * * * * * *(xArr(i) - xArr(i - 1)) * (yArr(i) - yArr(i - 1)) * * * * * * * *Exit For * * * * * *End If * * * *Next i * *End If End Function From one point of view, I would say yes, because a single subroutine should just do a single thing, as someone on this ng *recently reminded me (don't remember who). On the other hand, if one follows this principle too strictly, wouldn't he end writing "unsafe" code? If I take out the check from Interp2 and move it to another subroutine which the caller has to call before Interp2, *I risk forgetting to add the call to the checker subroutine when I reuse Interp2 in another code. Sure, I could place the call to the checking sub inside Interp2: however, this way I end up having a code with the same functionalities as before, but with the added slowdown due to the call to the checking subroutine. Best Regards Sergio |
#5
Posted to microsoft.public.excel.programming
|
|||
|
|||
A coding style question
Hi, Rick,
thanks for the reply, I'll do this way, as also suggested by Chip. Best Regards On 4 Feb, 19:09, "Rick Rothstein" wrote: If you mean the test in your If..Then statement... no, I would *not* move it to a separate subroutine. Personally, I think the the principal of one action per subroutine kind of overdoes it sometimes, but your question would not really fall under this principal as I understand it. -- Rick (MVP - Excel) "deltaquattro" wrote in message ... Hello, taking the cue from the previous thread I opened ("Error handling in VBA"), I would like to ask you if in your opinion it's usually better to check input data to a subroutine, inside the subroutine, or to move each check to a distinct subroutine. As an example, I'll post again my Interp subroutine, this time with some modifications added by Peter T: Public Function Interp2(xArr() As Double, yArr() As Double, _ * * * * * * * * * * * * * *x As Double) As Double Dim i As Long ' this is the part ... * *If ((x < xArr(LBound(xArr))) Or (x xArr(UBound(xArr)))) Then * * * *'MsgBox "Interp2: x is out of bound" & "X = " & x '.... I am considering moving outside the subroutine * *Else * * * *For i = LBound(xArr) To UBound(xArr) * * * * * *If xArr(i) = x Then * * * * * * * *Interp2 = yArr(i) * * * * * * * *Exit For * * * * * *ElseIf xArr(i) x Then * * * * * * * *Interp2 = yArr(i - 1) + (x - xArr(i - 1)) / _ * * * * * * * * * * * * *(xArr(i) - xArr(i - 1)) * (yArr(i) - yArr(i - 1)) * * * * * * * *Exit For * * * * * *End If * * * *Next i * *End If End Function From one point of view, I would say yes, because a single subroutine should just do a single thing, as someone on this ng *recently reminded me (don't remember who). On the other hand, if one follows this principle too strictly, wouldn't he end writing "unsafe" code? If I take out the check from Interp2 and move it to another subroutine which the caller has to call before Interp2, *I risk forgetting to add the call to the checker subroutine when I reuse Interp2 in another code. Sure, I could place the call to the checking sub inside Interp2: however, this way I end up having a code with the same functionalities as before, but with the added slowdown due to the call to the checking subroutine. Best Regards Sergio |
#6
Posted to microsoft.public.excel.programming
|
|||
|
|||
A coding style question
It is a generally accept princiiple of good programming never to do
the same thing in two different places (except for the most trival things). If you have a block of code in two or more locations, then you should consider taking that code, putting it in its own procedure, and the call that procedure when necessary. In your example, this would imply that you should leave the data validation code in Interp2, where it is immediately needed. However, if you find that as you write and expand the application, you are using the same code many times in many different procs, then it is time to think about moving the validation code to its own procedure. There are no hard and fast rules for making that decision. Generally, it boils down to personal coding style. Minimizing duplicate code has many advantages (and very few, if any, real disadvantages). First and foremost is that you need to write and test the code once, and once tested, it can be called from any procedure by just calling the function, no rewrites necessary. It also makes debugging, ehancement, and maintenance tasks much easier since you (or the person who inherits the from you) only need to revise on piece of code, rather than trying to hunt down all the occurrences and their variations. I tend to think of much of the code tasks to be similar to building something out of Lego blocks. Over time, you accumulate a standard library of modules and functions related to a task, say, for example, working with the System Registry. I have a module that contains all sorts of code for working with the registry. If I need registry functionality in an application, I just import the modRegistry module, and that's the end it. No additional coding or testing is required. I have around 200 VBA module designed to carry out a large variety of tasks, and the ability to just import a module that is self-contained and tested can save many, many, hours during developement. Cordially, Chip Pearson Microsoft MVP 1998 - 2010 Pearson Software Consulting, LLC www.cpearson.com [email on web site] On Fri, 5 Feb 2010 00:32:53 -0800 (PST), deltaquattro wrote: Hi Chip! Thanks for the reply, and compliments for your website, it's very good and full of interesting information (actually, it was the first site I used to start learning VBA for Excel). If I understood you correctly, you're saying that input data checking is a subtask of the sub itself, so I would do well to leave the check in Interp2. If the check was more complex, and used by many subroutines, I should consider moving it to a separate sub CheckData, and have the various subroutines such as Interp2 call CheckData, so I'm sure that each time Interp2 is called, CheckData is called too. Very well, I'll keep that suggestion in mind. Thank you for your help, Best Regards Andrea On 4 Feb, 19:25, Chip Pearson wrote: Broadly speaking, the validation of input data to a procedure should be within the procedure itself, and should execute before any other code. However, it might be the case that several different procedures have the same validation needs. For example, you may need to validate whether a server share is available, and do this validation in a number of independent procedures. In this case, you would probably want to move the actual data validation to its own proc and call that proc whenever the validation is required. *If multiple procedures need the same validation and that validation is complex, then you should put it in its own proc that returns a Boolean indicating whether the data is valid. There is no benefit, and there is some (very small) cost, to moving data validation code out of the proc that needs it to another proc. Taking it out to another proc just for the sake of taking it out buys you nothing. Whoever told you that a single proc should do a single thing was correct and you would do well to follow that recommendation. However, in this context, I would consider any input validation to be a subordinate task of the primary task carried out by the proc. Cordially, Chip Pearson Microsoft MVP 1998 - 2010 Pearson Software Consulting, LLCwww.cpearson.com [email on web site] On Thu, 4 Feb 2010 09:59:34 -0800 (PST), deltaquattro wrote: Hello, taking the cue from the previous thread I opened ("Error handling in VBA"), I would like to ask you if in your opinion it's usually better to check input data to a subroutine, inside the subroutine, or to move each check to a distinct subroutine. As an example, I'll post again my Interp subroutine, this time with some modifications added by Peter T: Public Function Interp2(xArr() As Double, yArr() As Double, _ * * * * * * * * * * * * * *x As Double) As Double Dim i As Long ' this is the part ... * *If ((x < xArr(LBound(xArr))) Or (x xArr(UBound(xArr)))) Then * * * *'MsgBox "Interp2: x is out of bound" & "X = " & x '.... I am considering moving outside the subroutine * *Else * * * *For i = LBound(xArr) To UBound(xArr) * * * * * *If xArr(i) = x Then * * * * * * * *Interp2 = yArr(i) * * * * * * * *Exit For * * * * * *ElseIf xArr(i) x Then * * * * * * * *Interp2 = yArr(i - 1) + (x - xArr(i - 1)) / _ * * * * * * * * * * * * *(xArr(i) - xArr(i - 1)) * (yArr(i) - yArr(i - 1)) * * * * * * * *Exit For * * * * * *End If * * * *Next i * *End If End Function From one point of view, I would say yes, because a single subroutine should just do a single thing, as someone on this ng *recently reminded me (don't remember who). On the other hand, if one follows this principle too strictly, wouldn't he end writing "unsafe" code? If I take out the check from Interp2 and move it to another subroutine which the caller has to call before Interp2, *I risk forgetting to add the call to the checker subroutine when I reuse Interp2 in another code. Sure, I could place the call to the checking sub inside Interp2: however, this way I end up having a code with the same functionalities as before, but with the added slowdown due to the call to the checking subroutine. Best Regards Sergio |
Reply |
Thread Tools | Search this Thread |
Display Modes | |
|
|
Similar Threads | ||||
Thread | Forum | |||
vba coding question | Excel Programming | |||
Comma Style Question | New Users to Excel | |||
Coding Question | Excel Programming | |||
VBA Coding Question | Excel Programming | |||
coding question | Excel Programming |