Broken "Do Until" loop

cying

New Member
Joined
Jun 26, 2014
Messages
29
Hi. I have a macro that's supposed to check many individual accounts for a stock symbol, then see if there is enough of that stock symbol in the portfolio to sell or to see if there is enough cash to buy that stock symbol.

The part checking if there are enough shares to sell the amount I'm validating for is the part that's not working. If I try to validate the last account in the list of accounts, the macro checks column B indefinitely (even though the "END" is present). If I try to validate any account(s) other than the last one, it works fine.

It looks like this with the (assumed) malfunctioning part in red:

Code:
Sub Validate()
Dim TransactionType As String
Dim Symbol As String
Dim EstPrice As Double
Dim Confirm As String
Dim CurRelRow As Integer
CurRelRow = 0
Dim CurShare As Integer
CurShare = 0
Dim FidTotal, SchTotal, TDAtotal, OthTotal As Integer
 
FidTotal = 0
SchTotal = 0
TDAtotal = 0
OthTotal = 0
 
TransactionType = Range("R1")
If TransactionType = "" Then
    MsgBox "Please enter transaction type"
    Exit Sub
End If
Symbol = Range("R2")
If Symbol = "" Then
    MsgBox "Please enter symbol"
    Exit Sub
End If
EstPrice = Range("R3")
If EstPrice <= 0 Then
    MsgBox "Please enter an estimated price"
    Exit Sub
End If
 
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
'Validate Sell All
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
If TransactionType = "Sell All" Then
    Confirm = MsgBox("Are you sure you want to sell all?", vbYesNo, "Confirm")
    If Confirm = vbYes Then
        Range("B9").Select
        Do Until ActiveCell.Value = "END"
            Cells(ActiveCell.Row, 18) = ""
            If ActiveCell.Value <> "" Then
                CurRelRow = ActiveCell.Row
            End If
            If UCase(Cells(ActiveCell.Row, 3).Value) = UCase(Symbol) Then
                Cells(CurRelRow, 18) = Cells(CurRelRow, 18) + Cells(ActiveCell.Row, 6)
            End If
            ActiveCell.Offset(1, 0).Select
        Loop
    Else
        MsgBox ("Validation Aborted")
    End If
 
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
'Validate Sell
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
ElseIf TransactionType = "Sell" Then
    Range("B9").Select
    [COLOR=#ff0000]Do Until ActiveCell.Value = "END"
        CurShare = 0
        If Cells(ActiveCell.Row, 18) <> "" Then
            CurRelRow = ActiveCell.Row
            ActiveCell.Offset(1, 0).Select
            Do Until ActiveCell.Value <> ""
                If UCase(Cells(ActiveCell.Row, 3)) = UCase(Symbol) Then
                    CurShare = CurShare + Cells(ActiveCell.Row, 6)
                End If[/COLOR]
                If Cells(ActiveCell.Row, 18) <> "" Then
                    Cells(ActiveCell.Row, 18).Select
                    MsgBox ("Please enter shares at the correct row")
                    Exit Sub
                End If
                ActiveCell.Offset(1, 0).Select
            Loop
            If Cells(CurRelRow, 18) > CurShare Then
                Cells(CurRelRow, 18).Select
                MsgBox ("Shares oversold, please fix and validate again")
                Exit Sub
            End If
        End If
        ActiveCell.Offset(1, 0).Select
    Loop
 
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
'Validate Buy
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
Else
    Range("B9").Select
    Do Until ActiveCell.Value = "END"
        If Cells(ActiveCell.Row, 18) <> "" Then
            If ActiveCell = "" Then
                Cells(ActiveCell.Row, 18).Select
                MsgBox ("Please enter shares at the correct row")
                Exit Sub
            End If
            If Cells(ActiveCell.Row, 18).Value * EstPrice > Cells(ActiveCell.Row, 13).Value Then
                Cells(ActiveCell.Row, 18).Select
                MsgBox ("Amount bought exceeds available cash")
                Exit Sub
            End If
        End If
        ActiveCell.Offset(1, 0).Select
    Loop
End If
 
Range("R9").Select
Do Until Cells(ActiveCell.Row, 1) = "END"
    If ActiveCell <> "" Then
        If Len(Cells(ActiveCell.Row, 3)) = 9 Then
            If InStr(Cells(ActiveCell.Row, 3), "-") = 5 Then
                SchTotal = SchTotal + Cells(ActiveCell.Row, 18).Value
            Else
                TDAtotal = TDAtotal + Cells(ActiveCell.Row, 18).Value
            End If
        ElseIf Len(Cells(ActiveCell.Row, 3)) = 10 Then
            FidTotal = FidTotal + Cells(ActiveCell.Row, 18).Value
        Else
            OthTotal = OthTotal + Cells(ActiveCell.Row, 18).Value
        End If
       
    End If
    ActiveCell.Offset(1, 0).Select
Loop
Cells(4, 18) = SchTotal
Cells(5, 18) = FidTotal
Cells(6, 18) = TDAtotal
Cells(7, 18) = OthTotal
 
MsgBox ("Validation completed")
 
End Sub

Please help! Thanks in advance!
 

Excel Facts

Will the fill handle fill 1, 2, 3?
Yes! Type 1 in a cell. Hold down Ctrl while you drag the fill handle.
hello

Please post some sample data showing the set up on a worksheet.

Likely there are alternatives to looping through the data. Looping can be very inefficient when there are larges amounts of data. If the data is well set up (such as in simple, well organised tables like a database) efficient approaches are available: Excel has very powerful functionality built in for working efficiently with such data. So a different approach to the current one might be better.

regards
 
Upvote 0
table.jpg


I hope that dropbox image works. Basically, we use the macro to check what R1 says (buy/sell/sell all), then it goes down the list of many hundreds of accounts to check if there is a number in column R on the account row, and if there is it checks if that number, multiplied by the price in B2, exceeds the available cash (for buy). for sell, it should check if the quantity in column R exceeds the quantity in the account already.

full disclosure: i did not write the macro, i'm merely attempting (poorly) to fix it
 
Upvote 0
Not what you want to hear but it really needs a total overhaul. It is poorly set up and the coding is poorly done. BTW, consequently I expect the code would be slow to run.

I'm not too interested in working with this but will take a look for a little while - but not too long - if you can post something usable. That is, not just an image.

Though the image of the worksheet helps to see the set up it doesn't help to work with it. I can't copy & paste from the image to have something to work with in Excel - all I end up with is an image, not entries in cells. Can you address that?

Though I may not get too involved in this, someone else may jump in & help: though I suspect not whilst there is just the image to work with.

regards
 
Upvote 0
4800Agg13.50%0.75%Sell
jpm59.79Gro10.80%0.70%jpm
Enh9.45%0.65%60
SCH21680Bal8.10%0.60%SCH845
FID24096Mod6.75%0.55%FID160
TDA370Inc5.40%0.50%TDA0
OTHERS0OTHERS0
GroupPortSymbolPctDateQuantityUnit CostTotal CostPriceMkt ValG&LPort TotalCash Avai.TargetVariationUnit AmtEst. SharesCASHEXPOSUREShort-Term?Restriction?Cash kept
account108/27/14-jf449,28013.43%3,345,881111,90513.50%2,41425,0944203.3%
account1axxxx-xxxx426,8772527728.51
83,137Agg GroNon-Taxable3.3%16.9%#N/A#N/A#N/A#N/A
stock11.29/10/20122,5001025,9581640,82514,867
stock20.71/30/20136523815,43733621,8716,433
stock31.22/11/20134755425,7228238,85513,133
stock40.910/12/20126003923,4695231,2787,809
stock50.85/14/20125003015,2165125,35510,139
stock60.910/27/20108944338,2773530,888(7,389)
stock70.98/11/201017512521,91117831,1339,222
stock80.72/11/20134505625,0625424,111(951)
stock90.78/13/201012511614,50018923,6749,173
jpm0.33/15/2013150507,531608,9611,430
stock110.95/25/2010400218,2447730,80022,556
stock121.02/25/20131,0002222,1133434,40012,287
stock130.68/16/20134753114,6344420,7206,086
stock141.07/2/20103004914,77411033,07218,298
stock150.92/13/20136003621,4835230,9369,454
account1bxxx-xxxxxx22,403554,85827,920Agg GroNon-Taxable5.0%4.0%#N/A#N/A#N/A#N/A
jpm0.77/2/20103753814,2266022,4038,177
account1cxxxx-xxxx0263,294849Agg GroTaxable0.3%0.0%#N/A#N/A#N/A#N/A
account20#DIV/0!0013.50%000#DIV/0!
account2axxx-xxxxxx000Agg GroNon-Taxable#DIV/0!#DIV/0!#N/A#N/A#N/A#N/A
account364,2447.21%891,37922,9788.10%7,9585,348902.6%
account3axxx-xxxxxx64,244891,37922,978BalTaxable2.6%7.2%#N/A#N/A#N/A#N/A
stock11.56/11/2009165254,0758213,4979,422
stock21.36/8/2009125415,1219111,3886,266
stock30.76/8/2009351485,1751786,2271,051
jpm1.06/10/2009150355,286608,9613,675
stock51.66/8/2009180173,0127713,86010,848
stock61.26/10/2009200255,0125210,3125,300

<tbody>
</tbody>


Thanks for your time and effort Fazza! I chose a screenshot because the copy/paste looked too hectic. But I guess it makes sense to copy/paste here, so someone can put it back into Excel. Yes indeed the code is very slow to run! Unfortunately, I am the most knowledgeable person in visual basic in the office, and I only took one C++ class in high school years ago.
 
Last edited:
Upvote 0
Had a look just now. You can too if you debug the code. If interested suggest you google for how to. Basically, within VBA the F8 button single steps through code. Easy if you have two monitors. Excel on one & VBA on other.

Anyway the code gets to the "Sell" section and starting from cell B9 loops & loops & loops down the rows. It is looking for an entry in column R (below R9) and loops until it finds something. As the sample data is empty in column R it doesn't stop. Just keeps looping.

From my quick look & the data provided, I don't know what to change. As in is the code wrong & the sample data correct, or the code correct & the data doesn't match what is expected, or both wrong.

HTH. Regards
 
Upvote 0
cying,

I took a look at the above procedure and rewrote the first portion to make it more concise. In your procedure the value 0 is assigned to a number of variables. This is not necessary as they start at 0.

Code:
Sub Validate()

 [COLOR=#0000ff]   Dim[/COLOR] TransactionType [COLOR=#0000ff]As String[/COLOR]
  [COLOR=#0000ff]  Dim [/COLOR]Symbol         [COLOR=#0000ff] As String[/COLOR]
[COLOR=#0000ff]    Dim[/COLOR] Confirm       [COLOR=#0000ff]  As String[/COLOR]
[COLOR=#0000ff]    Dim[/COLOR] EstPrice      [COLOR=#0000ff]  As Double[/COLOR]
[COLOR=#0000ff]    Dim [/COLOR]CurRelRow      [COLOR=#0000ff] As Integer[/COLOR]
  [COLOR=#0000ff]  Dim [/COLOR]CurShare        [COLOR=#0000ff]As Integer[/COLOR]
 [COLOR=#0000ff]   Dim[/COLOR] FidTotal      [COLOR=#0000ff]  As Integer[/COLOR]
[COLOR=#0000ff]    Dim[/COLOR] SchTotal        [COLOR=#0000ff]As Integer[/COLOR]
 [COLOR=#0000ff]   Dim[/COLOR] TDAtotal        [COLOR=#0000ff]As Integer[/COLOR]
  [COLOR=#0000ff]  Dim [/COLOR]OthTotal       [COLOR=#0000ff] As Integer[/COLOR]
    
    TransactionType = Range("R1")
    Symbol = Range("R2")
    EstPrice = Range("R3")
    
   [COLOR=#0000ff] If [/COLOR]TransactionType = "" [COLOR=#0000ff]Or [/COLOR]Symbol = "" [COLOR=#0000ff]Or[/COLOR] EstPrice <= 0 [COLOR=#0000ff]Then[/COLOR]
        MsgBox "Please check cells R1, R2 & R3 to make sure" & vbLf & _
        "the values are not missing or below zero.", vbCritical, "Please Check Data"
[COLOR=#0000ff]        Exit Sub[/COLOR]
[COLOR=#0000ff]    End If[/COLOR]
I also had a look at the portion you think is broken:
Code:
[COLOR=#008000]    ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''[/COLOR]
[COLOR=#008000]    'Validate Sell[/COLOR]
[COLOR=#008000]    ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''[/COLOR]
    [COLOR=#0000ff]ElseIf[/COLOR] TransactionType = "Sell"[COLOR=#0000ff] Then[/COLOR]
        Range("B9").Select   
      [COLOR=#0000ff]  Do Until[/COLOR] ActiveCell.Value = "END"   [COLOR=#008000]'Go through Column B until the word "END" is found then Stop[/COLOR]
            CurShare = 0                      [COLOR=#008000]   'This line resets the CurShare Value for each Specific Account[/COLOR]
          [COLOR=#0000ff]  If[/COLOR] Cells(ActiveCell.Row, 18) <> "" [COLOR=#0000ff]Then[/COLOR]
                CurRelRow = ActiveCell.Row
                ActiveCell.Offset(1, 0).Select
               [COLOR=#0000ff] Do Until[/COLOR] ActiveCell.Value <> ""
                   [COLOR=#0000ff] If [/COLOR]UCase(Cells(ActiveCell.Row, 3)) = UCase(Symbol) [COLOR=#0000ff]Then[/COLOR]  [COLOR=#008000]'Looks in Column C for JPM[/COLOR]
                        CurShare = CurShare + Cells(ActiveCell.Row, 6)         [COLOR=#008000]  'If JPM is found it adds to the Quantity in Column F in your example 150[/COLOR]
                   [COLOR=#0000ff] End If  [/COLOR]                                        [COLOR=#008000]'If the Quantity in Column R is Greater then the sum of JPM in Column F[/COLOR] [COLOR=#008000]Then you get the Shares Oversold MsgBox[/COLOR]
                        Cells(ActiveCell.Row, 18).Select
                  [COLOR=#0000ff]  If[/COLOR] Cells(ActiveCell.Row, 18) <> "" [COLOR=#0000ff]Then  [/COLOR]                         [COLOR=#008000]'This checks to make sure there is not more then one value in Column R for a specified account[/COLOR]
                        MsgBox ("Please enter shares at the correct row")     [COLOR=#008000]   '<---- If more than one value in Column R for an account then this MsgBox appears[/COLOR]
[COLOR=#0000ff]                        Exit Sub[/COLOR]
[COLOR=#0000ff]                    End If[/COLOR]
                    ActiveCell.Offset(1, 0).Select
[COLOR=#0000ff]                Loop[/COLOR]
               [COLOR=#0000ff] If[/COLOR] Cells(CurRelRow, 18) > CurShare T[COLOR=#0000ff]hen[/COLOR]
                    Cells(CurRelRow, 18).Select
                    MsgBox ("Shares oversold, please fix and validate again")    [COLOR=#008000]' <------If the SUM of Column F exceeds the value in Column R you will get this MsgBox[/COLOR]
[COLOR=#0000ff]                    Exit Sub[/COLOR]
[COLOR=#0000ff]                End If[/COLOR]
[COLOR=#0000ff]            End If[/COLOR]
            ActiveCell.Offset(1, 0).Select
[COLOR=#0000ff]        Loop                                    [/COLOR][COLOR=#008000]  'After One Account is Processed it will go to the next account in Column C until it hits "END"[/COLOR]
The code does not do this:
check if there is a number in column R on the account row, and if there is it checks if that number, multiplied by the price in B2, exceeds the available cash (for buy)
It does not multiply anything. It simply adds up the Sum Qty. of JPM in Column F (In the above example it is 150) and then checks to see if this number is greater then the corresponding number in column R.

Please read the commented code above. Now that it is clear what the code is doing. Can you see the error in the logic? If so tells us what you would like the code to do that it is not currently doing. Then we can solve your issue :)

Hope this helps.
 
Last edited:
Upvote 0
Anyway the code gets to the "Sell" section and starting from cell B9 loops & loops & loops down the rows. It is looking for an entry in column R (below R9) and loops until it finds something. As the sample data is empty in column R it doesn't stop. Just keeps looping.

Yes Definitely Fazza. I only included a small amount of accounts in the sample, and forgot to include the very bottom, which has END in both column A and B. The macro is supposed to search for "END" in the active cell but for some reason does not stop there for the SELL part (it works fine for buy)

It does not multiply anything. It simply adds up the Sum Qty. of JPM in Column F (In the above example it is 150) and then checks to see if this number is greater then the corresponding number in column R.

Yes the multiplication part was for the BUY part of the code, not the SELL (which is the part I'm having trouble with). First off, thanks for your less redundant code for the beginning!

The problem I'm having is that for the SELL part of the code, it ignores the:

Code:
ElseIf TransactionType = "Sell" Then
    Range("B9").Select
    Do Until ActiveCell.Value = "END"

part of the code and goes on forever
 
Upvote 0
I also had a look at the portion you think is broken:
Code:
[COLOR=#0000ff]        Loop                                    [/COLOR][COLOR=#008000]  'After One Account is Processed it will go to the next account in Column C until it hits "END"[/COLOR]
[/QUOTE]

mrmmickle1,

This is possibly where the macro is not working! Why does it search for END in column C? That column is blank in the data set, but there is END in both columns A and B. I'm pretty sure the macros for SELL ALL and BUY search for column B

EDIT: adding an END in column C does not fix the problem though

Code:
Do Until ActiveCell.Value <> ""
                If UCase(Cells(ActiveCell.Row, 3)) = UCase(Symbol) Then     'Looks in Column C for JPM
                    CurShare = CurShare + Cells(ActiveCell.Row, 6)          'If JPM is found it adds to the Quantity in Column F
                End If                                                      'If the Quantity in Column R is Greater then the sum of JPM in Column F Then you get the Shares Oversold MsgBox
                Cells(ActiveCell.Row, 18).Select
                If Cells(ActiveCell.Row, 18) <> "" Then                     'This checks to make sure there is not more then one value in Column R for a specified account
                    MsgBox ("Please enter shares at the correct row")       '<---- If more than one value in Column R for an account then this MsgBox appears
                    Exit Sub
                End If
                ActiveCell.Offset(1, 0).Select
            Loop

Using F8 (thanks Fazza!), this is the loop that keeps going
 
Last edited:
Upvote 0

Forum statistics

Threads
1,213,527
Messages
6,114,148
Members
448,552
Latest member
WORKINGWITHNOLEADER

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Back
Top