Go Premium for a chance to win a PS4. Enter to Win

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 243
  • Last Modified:

can the code be better?

I have this code and I just want to know if this is good coding or bad and if bad please show me how to make it better. and tell me why.  I have more versions of this to make and I want it the best that I can make it.
Thanks
Dim i As Integer
ReDim i2a(4) As integer
Dim i2 As integer
Dim u As string
Dim c As integer


i = 1
i2 = 1
Do While i <= Val(vTRUST_uInstance("Count"))
      If vTRUST_Name(i) <> "" Then
            u = vTRUST_uInstance(i)
            If    vREVOCABLE_TRUST_GRANTOR_zzTrustInstance(i2)  = u Then
                  Do While vREVOCABLE_TRUST_GRANTOR_zzTrustInstance(i2) = u
                        If vREVOCABLE_TRUST_GRANTOR_Name(i2) <> "" Then
                              c=c+1
                              i2a(c) = i2
                              If c = UBound(i2a) Then
                                    i2=i2+1
                                    Exit Do
                              End If
                        End If
                        i2=i2+1
                  Loop
            End If
            
            If c > 0 Then
                  If c < UBound(i2a) Then
                        i=i+1
                  End If
                  For c = 1 To UBound(i2a)
                        If i2a(c)= 0 Then
                              i2a(c)=99
                        End If
                  Next
                  
                  oRequest.SetValue "Trust_Index", "to", i
                  oRequest.SetValue "grantor_Index", "to", i2a(1)
                  oRequest.SetValue "grantor2Index", "to", i2a(2)
                  oRequest.SetValue "grantor3Index", "to", i2a(3)
                  oRequest.SetValue "grantor4Index", "to", i2a(4)
                  oTemplate.LoadFormData
                  oForm.PrintTag = "File"
                  oForm.NoOfCopies = 1
                  oForm.RelativePosition = 1
                  AddFormToPackage
                   oRequest.SetValue "Trust_Index", "to", "" 
                  oRequest.SetValue "grantor_Index", "to", ""
                  oRequest.SetValue "grantor2Index", "to", ""
                  oRequest.SetValue "grantor3Index", "to", ""
                  oRequest.SetValue "grantor4Index", "to", ""
                  C = 0
                  ReDim i2a(4)
                  
            Else
                  i = i + 1
            End If
      Else
            i = i + 1
      End If
Loop
0
cdb424ttm
Asked:
cdb424ttm
2 Solutions
 
2AngelCommented:
Hi,

Did you leartnt Algorithmics?

I don't know how to wirte the explanation in English but, for a good code you have to calculate the running time and the system usage (CPU, Memory and so).

You are using loops - for a better usage and run-time coding, using a recursive is much better!!!



Have fun
2Angel
0
 
Arthur_WoodCommented:
without knowing what it is you are trying to accomplish with this code, it is very difficult to tell you whether the code is 'good' or 'bad'.

Can you explain, in English, what it is you want the code to do?

AW
0
 
bramsquadCommented:
two things ill disagree with about the first statement.

1)  recursion is a function calling itself.  i see no where in this code you could incorperate any recursive methods.  

2)  recursive methods (like the fibonacci sequence) have a growth rate of 2^n, which is terrible.  if there is any depth in the program whatsoever its going to take forever to execute.

sorry to call you out, but recursion is something that is and only should be used when it has to be.

as far as your code goes, the only thing that could make your program lag is your nested loops.  thing is, is sometmies you have to do it that way.  

but yea, id say explain what your doing to give us a better understanding of whats going on

~b
0
Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
Jaime OlivaresCommented:
You can be a little faster in these situations

Do While i <= Val(vTRUST_uInstance("Count"))
' etcetera

             For c = 1 To UBound(i2a)
             'etcetera

You are evaluating to a funtion result every time you loop. If you invert process order (if possible) you will achieve a better performance. By example: it will be faster

             For c = UBound(i2a) to 1 step -1

Because UBound(i2a) will evaluate only once.
0
 
Jaime OlivaresCommented:
Maybe you can avoid the second    ReDim i2a(4)
0
 
Arthur_WoodCommented:
one way to speed up loops that have a Function call to determine the limits is to set a variable to the value returned by a SINGLE call to the function, and then use that varaible as the limit for the loop:

Dim uLimit as Integer
uLimit = UBound(ia2)
for c = 1 to uLimit


instead of

for c = 1 to UBound(ia2)


or


Dim iVal as Integer
iVal = Val(vTRUST_uInstance("Count"))
Do While i <= iVal

instead of :

Do While i <= Val(vTRUST_uInstance("Count"))

see how it works.

It also makes it possible to debug the code, by setting a breakpoint and then single stepping to the line in question.  If yiou hover the mouse-pointer over the variable, a 'tool-tip' will show the current value of the variable, or you can print the value in the Immedaite Window using CTRL-G, and then type

? iVal

or

?uLimit

and press the enter key.  The value will then be printed in the Immedaite Window.

AW


0

Featured Post

Keep up with what's happening at Experts Exchange!

Sign up to receive Decoded, a new monthly digest with product updates, feature release info, continuing education opportunities, and more.

Tackle projects and never again get stuck behind a technical roadblock.
Join Now