Solved

can the code be better?

Posted on 2004-07-30
6
236 Views
Last Modified: 2010-05-02
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
Comment
Question by:cdb424ttm
6 Comments
 
LVL 2

Expert Comment

by:2Angel
ID: 11680306
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
 
LVL 44

Expert Comment

by:Arthur_Wood
ID: 11680359
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
 
LVL 8

Expert Comment

by:bramsquad
ID: 11680776
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
Is Your Active Directory as Secure as You Think?

More than 75% of all records are compromised because of the loss or theft of a privileged credential. Experts have been exploring Active Directory infrastructure to identify key threats and establish best practices for keeping data safe. Attend this month’s webinar to learn more.

 
LVL 55

Assisted Solution

by:Jaime Olivares
Jaime Olivares earned 250 total points
ID: 11681502
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
 
LVL 55

Expert Comment

by:Jaime Olivares
ID: 11681525
Maybe you can avoid the second    ReDim i2a(4)
0
 
LVL 44

Accepted Solution

by:
Arthur_Wood earned 250 total points
ID: 11682679
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

Is Your Active Directory as Secure as You Think?

More than 75% of all records are compromised because of the loss or theft of a privileged credential. Experts have been exploring Active Directory infrastructure to identify key threats and establish best practices for keeping data safe. Attend this month’s webinar to learn more.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

The debugging module of the VB 6 IDE can be accessed by way of the Debug menu item. That menu item can normally be found in the IDE's main menu line as shown in this picture.   There is also a companion Debug Toolbar that looks like the followin…
You can of course define an array to hold data that is of a particular type like an array of Strings to hold customer names or an array of Doubles to hold customer sales, but what do you do if you want to coordinate that data? This article describes…
Get people started with the process of using Access VBA to control Outlook using automation, Microsoft Access can control other applications. An example is the ability to programmatically talk to Microsoft Outlook. Using automation, an Access applic…
This lesson covers basic error handling code in Microsoft Excel using VBA. This is the first lesson in a 3-part series that uses code to loop through an Excel spreadsheet in VBA and then fix errors, taking advantage of error handling code. This l…

911 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

23 Experts available now in Live!

Get 1:1 Help Now