1

I needed to write some code that would select the minimum of blocks of cells that correspond to equal cell values across rows in another column. Here's what I have:

Function MoveDown(c)
    MoveDown = c.Offset(0, 1).Select
End Function

Sub LoanOptimization()
    For Each c In Worksheets("Sheet1").Range("C1:C65536").Cells
        c1 = c.Row
        Do While c.Value = MoveDown(c).Value
            c = MoveDown(c)
            c2 = MoveDown(c).Row
            Set CellRange = ActiveSheet.Range(Cells(c1, 12), Cells(c2, 12)).Select
            Minimum = Application.WorksheetFunction.Min(CellRange)
        Loop
    Next
    Range("N3").Select
    ActiveSheet.Paste
End Sub
  1. Does this even make sense for what I'm trying to do, and
  2. How do I paste the values from my Minimum variable into another column

EDIT:

I changed and added to my code, because my first question was just the first step in what I needed to do. Here is what I have, but I keep getting compile errors:

Dim lastRow As Integer
Dim i As Integer
Dim comp1 As Integer
Dim comp2 As Integer
Dim rngCount As Integer
Dim minimum As Integer
Dim comp3 As Integer
Dim comp4 As Integer

lastRowDate = WorksheetFunction.CountA(Range("G:G")) 'Find the last row with data
lastRowNotional = WorksheetFunction.CountA(Range("L:L"))
rngCount = 0

For i = 1 To lastRowDate
    comp1 = ActiveSheet.Cells(i, 7).Value 'Set comp1 equal to the Value of the cell in         Column C at the current row in the For loop
    comp2 = ActiveSheet.Cells(i + 1, 7).Value 'Set comp2 equal to the value of the cell just below it

    If comp1 <> comp2 Then 'If the values are different, i.e. we've found the last item in a series of matches
        minimum = Application.WorksheetFunction.Min(Range(Cells(i, 12), Cells(i - rngCount, 12))) 'Find the minimum of the range of cells
        'from Row i in Column D to Row i - rngCount (which is were our series of matches began)
        Cells(i, 14).Value = minimum 'Paste the found minimum in Column N, Row i
        rngCount = 0 'Because the values no longer match, reset our counter
        For j = 1 To lastRowNotional

        comp3 = ActiveSheet.Cells(i, 14)
        comp4 = ActiveSheet.Cells(i + 1, 14)
        offset1 = ActiveSheet.Cells(i - 1, 14)

            If (comp3 = offset1) And (comp3 <> comp4) Then 'If the selected cell is the last in a block of minima,
        'then we want to replace that cell only with sum of the values in that block
                Summation = Application.WorksheetFunction.Sum(Range(Cells(i, 12), Cells(i - rngCount, 12)))
            Cells(i, 14).Value = Summation
            End If
    Else 'If the values are the same
        rngCount = rngCount + 1 'increment our range counter until the values do not match
    End If
Next i
Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
jovianlynxdroid
  • 335
  • 3
  • 5
  • 14
  • See [this](http://stackoverflow.com/questions/20738373/can-i-make-this-macro-more-efficient-or-faster/20754562#20754562) for some basic tips. – WGS Jan 10 '14 at 04:24
  • Also, I'm thinking you are making this more complicated than it is. Can you describe exactly what you want to do? I think this is just getting the minimum value in a specified range of cells, yes? – WGS Jan 10 '14 at 04:30
  • If the range of cells from C5:C10 are the same, then I want to get the minimum of the block from L5:L10 and paste all of those in a new column. And so on and so forth... – jovianlynxdroid Jan 10 '14 at 13:41

1 Answers1

0

To start, the best way to learn VBA is to start the macro recorder, do what you want in the GUI and then look at the resulting code. I would recommend doing that as much as possible.

To paste your value in cell N3, use

Range("N3").Value = Minimum 'Or whatever you want to paste

In the code you provided, you never copied anything. If you have the value you want selected, you can use

Selection.Copy

to copy it and then paste as you did in your code. In your range, you can use Range("C:C") to select the entire column instead of going to row 65536. This should get your code working. As a note, I probably wouldn't bother making Movedown its own function. It's one line, so you could just use it explicitly.

I have not tested this code, but if it was working other than the pasting portion, this should fix it. If you're not sure where it's having trouble, try using

MsgBox c 'or any text or variable name

This will give you a dialog box showing the value of your variable at the point you added the MsgBox line. This is often helpful to determine if your variable isn't being set properly or if there is an issue with pasting or displaying it.

Hope this helps! Good luck!


EDIT:

Here's code that should do what you want. I commented heavily, so hopefully it's clear what is happening, but let me know if not. The basic idea is there's a For loop that is checking the current value of the cell in Column C against the cell just below it and if they match, it increments a variable and keeps track of how many of the same value are in a row. As soon as it finds cells that don't match, it finds the minimum of the range of the current cell (in Column D) through the first cell that matched (which is why we were keeping track of how many matched in a row.)

I didn't explain earlier, but if you didn't know the single apostrophe is a line comment character in VBA. That means anything appearing after the apostrophe is not read by the program and is just for humans to keep track of what is happening.

Private Sub findMin()
Dim lastRow As Integer
Dim i As Integer
Dim comp1 As Integer
Dim comp2 As Integer
Dim rngCount As Integer
Dim minimum As Integer

lastRow = WorksheetFunction.CountA(Range("C:C")) 'Find the last row with data
rngCount = 0

For i = 1 To lastRow

    comp1 = ActiveSheet.Cells(i, 3).Value 'Set comp1 equal to the Value of the cell in Column C at the current row in the For loop
    comp2 = ActiveSheet.Cells(i + 1, 3).Value 'Set comp2 equal to the value of the cell just below it

    If comp1 <> comp2 Then 'If the values are different, i.e. we've found the last item in a series of matches
        minimum = Application.WorksheetFunction.Min(Range(Cells(i, 4), Cells(i - rngCount, 4))) 'Find the minimum of the range of cells from Row i in Column D to Row i - rngCount (which is were our series of matches began)
        Cells(i, 14).Value = minimum 'Paste the found minimum in Column N, Row i
        rngCount = 0 'Because the values no longer match, reset our counter
    Else 'If the values are the same
        rngCount = rngCount + 1 'increment our range counter until the values do not match
    End If
Next i

End Sub

A few notes in addition to what I said above about best practices: Avoid using Select unless you have a good reason to use it. There's a number of reasons why and some lengthy discussions about it on this site and others, but for now suffice it to say that it adds length and possible confusion that is unnecessary. Cells(1,1).Select: Selection.Value = variableA is the same as Cells(1,1).Value = variableA1. I would also avoid using Copy and Paste for similar reasons when you can use .Value =. This is clearer to read and has fewer chances for things to go wrong or not work as you intended. I would also recommend that you comment your code heavily, especially if you are having trouble and paste it here. This will help others better understand what you were trying to do. Even if it works fine, it is a good idea to comment in case you need to change it months later or someone else needs to read it. It's a good habit to get in.


EDIT 2:

This should be what you're looking for. I tried to comment the changes I made, so hopefully it makes sense. As far as the If statement in For j goes, you had it set to check if cell j matched the cell above it and if it was different from the cell below it. However, in Column N, the first For loop only puts code on cells where the value is different. Whenever the value is the same, the corresponding cell in Column N is blank. So checking to see if Column N has a positive value in it will catch cells with data and ignore blank cells. You could also only check if the next cell is different. If you check this on every single cell, it can safely be assumed that the previous cell was the same. This is what I did in the first For loop.

Private Sub findMin()

Dim lastRow As Integer
Dim i As Integer
Dim comp1 As Integer
Dim comp2 As Integer
Dim rngCount As Integer
Dim minimum As Integer
Dim comp3 As Integer
Dim comp4 As Integer
Dim lastRowNotional As Integer
Dim j As Integer
Dim offset1 As Integer
Dim summation As Double 'I don't know how many items you are summing or how large they are, but if it's too large Integer won't work.  I needed to use Double for the sample data I made up

lastRowDate = WorksheetFunction.CountA(Range("G:G")) 'Find the last row with data
lastRowNotional = WorksheetFunction.CountA(Range("L:L")) 'Unless Columns G and L are different lengths, there is not a need to have a second variable
rngCount = 0

For i = 1 To lastRowDate
    comp1 = ActiveSheet.Cells(i, 7).Value 'Set comp1 equal to the Value of the cell in Column C at the current row in the For loop
    comp2 = ActiveSheet.Cells(i + 1, 7).Value 'Set comp2 equal to the value of the cell just below it

    If comp1 <> comp2 Then 'If the values are different, i.e. we've found the last item in a series of matches
        minimum = Application.WorksheetFunction.Min(Range(Cells(i, 12), Cells(i - rngCount, 12)))
        Cells(i, 14).Value = minimum 'Paste the found minimum in Column N, Row i
        rngCount = 0 'Because the values no longer match, reset our counter

    Else: comp1 = comp2 'If the values are the same
         rngCount = rngCount + 1 'increment our range counter until the values do not match
    End If
Next i

'I moved this whole For loop outside the other one so that it doesn't try to run for every new i
 'Also, you had i inside this loop, but your loop counter is j.  This is an easy mistake to make when using a lot of For loops
    For j = 1 To lastRowDate 'We want to only check as many rows in Column N as we output, which is equal to lastRowDate

         comp3 = ActiveSheet.Cells(j, 14).Value 'I added the .Value here

         If Cells(j, 14).Value > 0 Then 'This will throw an error when it tries to find the cell above row 1. Be careful of using row - 1 on functions that include the first row
        'Just checking to see if the cell's value is greater than zero should suffice
             summation = Application.WorksheetFunction.Sum(Range(Cells(j, 12), Cells(j - rngCount, 12)))
             Cells(j, 15).Value = summation 'I moved this to paste in Column O.  Otherwise it would paste over the minimum we just found, defeating the purpose of finding the minimum
             rngCount = 0 ' Don't forget to reset your counter here.....
        Else
             rngCount = rngCount + 1 '... or increment it here

        End If
    Next j 'Be sure to include Next j to move the loop forward

End Sub
thunderblaster
  • 918
  • 11
  • 27
  • When I ran it, I got the error "Unable to get Select property of the range class", and it pointed to the line where the Do loop began – jovianlynxdroid Jan 10 '14 at 13:38
  • Check [this answer](http://stackoverflow.com/questions/20965009/is-it-possible-to-count-date-stamped-variables-accross-multiple-sheets-in-excel/20986392#20986392) I posted to a similar question. In that one, the question asked was trying to total values of Column F when Column E was the same. Instead of adding them, you would take the minimum, but the For loop should give you a good direction for checking if your column C values are the same. – thunderblaster Jan 10 '14 at 14:01
  • I'm sorry, I can't see how I should apply the parallel. I have int values for my C column so I can't define them as variables – jovianlynxdroid Jan 10 '14 at 14:09
  • Just edited my answer and included full working code. Let me know if anything is unclear or if it doesn't work how you wanted. – thunderblaster Jan 10 '14 at 15:22
  • I tried running it, and I got a type error pointing to this line: comp2 = ActiveSheet.Cells(i + 1, 7) – jovianlynxdroid Jan 10 '14 at 16:15
  • Be sure you have it as `comp2 = ActiveSheet.Cells(i + 1,7).Value` (with .Value after the Cells). As you have it written, you would be assigning the cell object to the variable and instead you want to set the value of the cell. – thunderblaster Jan 10 '14 at 16:44
  • I think I just copied it incorrectly. In my code, I have .Value after the Cells – jovianlynxdroid Jan 10 '14 at 16:49
  • If the code still isn't working, can you edit the question and add the code exactly as you have it? I tested the code I provided with some sample data and it was working correctly. Otherwise if it's fixed and complete, please accept the answer. – thunderblaster Jan 10 '14 at 16:58
  • Your comp1 and comp2 were fine, but comp3 and comp4 were missing the `.Value`. I changed a few other things to ensure it worked and made it a bit more straightforward. – thunderblaster Jan 10 '14 at 19:49
  • I don't understand how "If Cells(j, 14).Value > 0 Then" achieves the same thing as what I had written – jovianlynxdroid Jan 10 '14 at 19:55
  • Edited with further elaboration on that If statement. I googled the error you're getting and it may be that you have non-numeric data in Column G. This prevents the variable comp2 from taking on its value as comp2 can only save integers. The code I provided is working properly when I have only numbers in Columns G and L. If you're still getting errors, check your inputs on the spreadsheet. – thunderblaster Jan 10 '14 at 20:07
  • It's a Date, but I'm not sure if it is a Date variable (if there is such a thing in Excel) or an Integer. Do you know how to handle that? – jovianlynxdroid Jan 10 '14 at 20:27
  • There is. Change `Dim comp2 As Integer` to `Dim comp2 As Date`. – thunderblaster Jan 10 '14 at 20:35
  • I set both comp1 and comp2 as dates and it worked correctly when I had all dates in Column G. If it's still not working, it's something in the sheet and we'd need you to upload it to dropbox or similar in order to assist further. – thunderblaster Jan 10 '14 at 21:23
  • Okay, how do I share with you on dropbox? – jovianlynxdroid Jan 10 '14 at 21:25
  • I don't have a dropbox account, so I don't know. I believe you can make files public and paste the link. – thunderblaster Jan 10 '14 at 21:44
  • Alright. I tried running it again and I got an Else without If error, and you say the code runs when you try it out. So it may not only be an issue with the sheet – jovianlynxdroid Jan 10 '14 at 21:51
  • Check for a Next after each For and an End If after each If. If one of these is missing, it will sometimes give you that error (even if your Else comes after an If.) – thunderblaster Jan 10 '14 at 21:55
  • Fixed it. The type mismatch error persists. Here is my spreadsheet: https://skydrive.live.com/edit.aspx?cid=D3097819BB57146E&resid=D3097819BB57146E%21107&app=Excel&wdo=1 – jovianlynxdroid Jan 10 '14 at 22:06
  • It prompted for a login. Do you need to have a SkyDrive account to view it? Or is it prompting for your creds? – thunderblaster Jan 11 '14 at 17:55
  • It's public, so a skydrive account should do – jovianlynxdroid Jan 11 '14 at 18:43
  • I don't have a SkyDrive account. Perhaps try removing data from Column G to see if it executes properly. If comp1 doesn't throw an error, that shows that G1 is being handled properly. There's probably some cell in column g it doesn't like. You could use MsgBox to follow and see where it gets just before it throws the error. Put the MsgBox inside the For loop and have it display i. [Here's](http://msdn.microsoft.com/en-us/library/139z2azd(v=vs.90).aspx) a link if you aren't sure how to do that. – thunderblaster Jan 11 '14 at 21:22