Results 1 to 5 of 5
  1. #1
    Join Date
    Sep 2009
    Posts
    11

    Question Unanswered: Stop a Macro Running twice

    I have written a fairly long procedure that allows you select a file to open, opens it, does some work on the data in the file and then copies the data to the first file.

    It also numbers each piece of data you input and allows you to put a brief description of the data before pasting it to the first file.

    It then closes the file that you extracted the data from.

    I have made separate subs for each task and use the call function to link the subs together.

    It all works properly but for some reason it always loops through the procedure twice. I have tried putting a pause in the last sub but that does not stop it running twice.

    The last sub does not call another sub and unless I am blind I have not accidently looped the macros earlier in the procedure.

    It always loops through the whole procedure twice and stops. How can I get the procedure to only run once and then stop?

    If it was not for the fact that by cancelling the second loop changes the count in the original file I would not worry too much but, to keep data in the original file correct I need it to stop at the end,

    Because the procedure is fairly long I Have not included it here. Do I need to include the all of the code or at least the first couple and last couple of subs?

  2. #2
    Join Date
    Sep 2008
    Location
    London, UK
    Posts
    511
    Hi,

    It's hard for us to speculate why it is running twice without seeing the code. So yes, please post all the code. One possibility which springs to mind is worksheet/workbook event handlers, but we'll have to wait and see...

  3. #3
    Join Date
    Sep 2009
    Posts
    11

    Question

    Colin

    Thanks. I managed to stop it running twice by putting it all into one big macro rather than having a whole lot of small subs and calling each one in turn,

    Someone once said it was better to have a serries of small macros linked rather than one big Macro and it is certainly easier to edit and debug smaller chunks.

    Should I try to fix the original problem or just use the large macro?

    Having got past this hurdle my next task is to copy the data to the clipboard. I have tried using the code

    Sub SelectColumn()
    Dim UpBound As Range
    Dim LowBound As Range

    If ActiveCell.Row > 1 Then
    If IsEmpty(ActiveCell.Offset(-1, 0)) Then
    Set UpBound = ActiveCell
    Else
    Set UpBound = ActiveCell.End(xlUp)
    End If
    Else
    Set UpBound = ActiveCell
    End If

    If ActiveCell.Row < Rows.Count Then
    If IsEmpty(ActiveCell.Offset(1, 0)) Then
    Set LowBound = ActiveCell
    Else
    Set LowBound = ActiveCell.End(xlDown)
    End If
    Else
    Set LowBound = ActiveCell
    End If

    Range(UpBound, LowBound).Select

    Set UpBound = Nothing
    Set LowBound = Nothing

    End Sub

    But it and other code which is available always select a whole heap of empty cells at the end of the data. I realise the problem is caused by my own sloppy programming in the first case although I use the formula as shown above to copy data from the source file to paste into the main file where the data is combined.

    How can I only select cells that actually have data in them?

    I have put the following formula ("=IF(A2="","",1)") in the next column and used the count function to calculate the number of rows I need to select but cannot work out how put that number into the macro.

    How do I write code that selects from A1:AXX where the XX is the number held in cell B1?

    Is it posible to write the formula so it calculates in vba rather than getting the number from the spreadsheet?



    John

  4. #4
    Join Date
    Sep 2008
    Location
    London, UK
    Posts
    511
    John,
    Someone once said it was better to have a serries of small macros linked rather than one big Macro and it is certainly easier to edit and debug smaller chunks.
    Yes, within reason, I try to break a large procedure down into smaller, logical components.

    How can I only select cells that actually have data in them?
    There are several ways to determine the last used row of a range - some of which are more reliable than others. My weapon of choice is this function:
    Code:
    Public Function Get_Last_Row(ByVal rngToCheck As Range) As Long
    
        Dim rngLast As Range
        
        Set rngLast = rngToCheck.Find(what:="*", searchorder:=xlByRows, searchdirection:=xlPrevious)
        
        If rngLast Is Nothing Then
            Get_Last_Row = rngToCheck.Row
        Else
            Get_Last_Row = rngLast.Row
        End If
        
    End Function
    You can pass in the cells of the entire worksheet, or particular areas, and it will tell you the last row containing data or a formula. eg.
    Code:
    Sub test()
        
        'what is the last used row in the entire sheet?
        MsgBox Get_Last_Row(Sheet1.Cells)
        
        'what is the last used row in the 2nd column?
        MsgBox Get_Last_Row(Sheet1.Columns(2))
        
        'what is the last used row in the range E1:N30?
        MsgBox Get_Last_Row(Sheet1.Range("E1:N30"))
        
    End Sub
    When you are automating a process in Excel using VBA, you will find that 99% of the time you don't need to activate or select objects to manipulate or use them. For example, if you want to copy range A1:A10, you can just copy it - there's no need to select it first.
    Code:
    Sub BadExample()
        
        'macro recorder style code
        Sheets("Sheet1").Select
        Range("A1:A10").Select
        Selection.Copy
        
    End Sub
    
    Sub GoodExample()
    
        'we can just use
        Worksheets("Sheet1").Range("A1:A10").Copy
        
    End Sub
    Selecting cells will introduce buggy behaviour into your code and slow it down so, as a rule of thumn, try to avoid doing it.


    Hope that helps...
    Last edited by Colin Legg; 10-05-09 at 05:33.

  5. #5
    Join Date
    Sep 2009
    Posts
    11
    Colin

    I could not get that public function to work but using the following code I managed to select only cells with data in them rather than all the empty but active cells I have managed to insert into the worksheet.

    Now I will try to work out how to opent the text file and paste the data into it automatically. I will also try to debug the original macros to stop them always running twice.

    Thanks for your help. I am sure I will be back asking for more help.

    Sub FindLastCell()

    'Deletes Active cells that have no data in them


    Application.ScreenUpdating = False
    Sheets("Sheet2").Select
    Range("A1").Select
    On Error Resume Next
    lRealLastRow = Cells.Find("*", Range("A1"), xlFormulas, , xlByRows, xlPrevious).Row
    lRealLastColumn = Cells.Find("*", Range("A1"), xlFormulas, , xlByColumns, xlPrevious).Column
    Cells(lRealLastRow + 1, lRealLastColumn).Select
    Range(Selection, ActiveCell.SpecialCells(xlLastCell)).Select
    Application.CutCopyMode = False
    Selection.ClearContents

    Call SelectColumn
    End Sub
    Sub SelectColumn()
    Dim UpBound As Range
    Dim LowBound As Range

    'Copies data to clipboard to insert into text file



    Application.ScreenUpdating = False

    Range("A1").Select
    If ActiveCell.Row > 1 Then
    If IsEmpty(ActiveCell.Offset(-1, 0)) Then
    Set UpBound = ActiveCell
    Else
    Set UpBound = ActiveCell.End(xlUp)
    End If
    Else
    Set UpBound = ActiveCell
    End If

    If ActiveCell.Row < Rows.Count Then
    If IsEmpty(ActiveCell.Offset(1, 0)) Then
    Set LowBound = ActiveCell

    Else
    Set LowBound = ActiveCell.End(xlDown)
    End If

    Range(UpBound, LowBound).Copy
    Else
    Set LowBound = ActiveCell
    End If

    Range(UpBound, LowBound).Select

    Set UpBound = Nothing
    Set LowBound = Nothing

    End Sub

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •