asked on

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

2Angel

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

Arthur_Wood

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

bramsquad

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

SOLUTION

Jaime Olivares

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial