Having random trouble with a line of VBA code

[Shiremaid]

So I have a macro to add a line to a database I've created by copying an existing line and inserting above then clearing the data.
Mostly it works fine, but every once in a while for no reason that I can tell, it will suddenly give me an error. Usually happens when I am running the macro a few times in a row.
I make sure that the macro has finished running before running it again (which freezes it rather than give me an error anyway).
I should also note that this error also freezes Excel (opening the code editor is the only thing that still works) so I have to restart.
Any thoughts?

This is the line that's giving me trouble (or at least it's the one that's highlighted when I hit "Debug")
Selection.Insert Shift:=xlDown

Full code below

Private Sub Worksheet_BeforeDoubleClick(ByVal Target As Range, Cancel As Boolean)
    If Target.Value = "Double-Click to Add An Action" Then
    ActiveCell.Offset(2, 0).Rows("1:1").EntireRow.Select
    Selection.Copy
    Selection.Insert Shift:=xlDown
    ActiveCell.Range("A1:F1").Select
    Selection.ClearContents
    ActiveCell.Select
    ElseIf Target.Value = "Double-Click to Add an Amendment" Then
    ActiveCell.Rows("1:3").EntireRow.Select
    Selection.Copy
    ActiveCell.Select
    Selection.End(xlDown).Select
    ActiveCell.Offset(1, 0).Rows("1:1").EntireRow.Select
    Selection.Insert Shift:=xlDown
    ActiveCell.Select
    ActiveCell.FormulaR1C1 = "Amendment"
    ActiveCell.Offset(0, 4).Range("A1").Select
    Selection.ClearContents
    ActiveCell.FormulaR1C1 = "HHS #"
    ActiveCell.Offset(0, -4).Range("A1").Select
    With Selection.Font
        .Color = -16776961
        .TintAndShade = 0
    End With
    ActiveCell.Offset(2, 0).Range("A1:E1").Select
    Selection.ClearContents
    End If
    Application.CutCopyMode = False
End Sub

Thanks

 

[Veritan]

Hi Shiremaid, I've got a couple ideas for you. To me, the first thing I would do is clean up the code. You have a lot of unnecessary lines in there. For instance, ActiveCell.Select does absolutely nothing. Since the active cell has already been selected, there's no point in trying to select it again. I'd delete all occurrences of that line. Second, VBA is a very forgiving programming language. It can infer a lot without you having to explicitly state something. Selecting data is a great example of this. In VBA, 90% of the time, you don't have to select or activate anything. In fact, it's better not to do so, as this action will slow your code down considerably. If I had to guess, I'd say all the selecting you're doing is probably your prime suspect for crashing and freezing. For instance, where you have

    ActiveCell.Offset(2, 0).Rows("1:1").EntireRow.Select
    Selection.Copy
    Selection.Insert shift:=xlDown

try replacing that with this:

    With ActiveCell.Offset(2)
        .EntireRow.Copy
        .EntireRow.Insert Shift:=xlDown
    End With

This avoids the selection method entirely, which can really speed up your code. Finally, I'd strongly suggest turning off screen updating by putting Application.ScreenUpdating = False as your very first line of code, and Application.ScreenUpdating = True as your very last line of code. Turning off screen updating allows VBA to do its work without Excel trying to show you every single intermediate step. It will only show you the final result, which is what you want. In any situation where you have a lot going on, this can speed up your code a lot. As a side note, even with screen updating turned off, if you're stepping through your code, Excel will still update with every line, so you'll be able to see what's happening when you're debugging. Try cleaning up the code first and seeing if it works. If you're still having problems, you can repost the new code and see if anybody can help with that.

 

[Shiremaid]

Ok, so I've cleaned up my code a bit and most of the workbooks I create from the new template seem to be ok so far (knock on wood) but I am still have trouble with the original database I created. Could there be something corrupt in the file somewhere? Or are there still trouble spots in my code? The workbook consistently crashes when I try to do a save as (which I need to do at least once a week) and still periodically gives me an error at the same line as before.
I also tried saving the workbook without the macros and then putting them back in and it still has the same problems.
Could size be the problem? This is the largest of the databases (currently at 3797 KB) and I'm usually ok if I go in, do one thing, save and exit.

Private Sub Worksheet_BeforeDoubleClick(ByVal Target As Range, Cancel As Boolean)
    Application.ScreenUpdating = False
    If Target.Value = "Double-Click to Add An Action" Then
        With ActiveCell.Offset(2)
            .EntireRow.Copy
            .EntireRow.Insert Shift:=xlDown
        End With
        ActiveCell.Offset(2, -1).Range("A1:F1").Select
        Selection.ClearContents
     ElseIf Target.Value = "Double-Click to Add an Amendment" Then
        ActiveCell.Rows("1:3").EntireRow.Select
        Selection.Copy
        Selection.End(xlDown).Select
        ActiveCell.Offset(1, 0).Rows("1:1").EntireRow.Select
        Selection.Insert Shift:=xlDown
        ActiveCell.FormulaR1C1 = "Amendment"
        ActiveCell.Offset(0, 4).Range("A1").Select
        ActiveCell.FormulaR1C1 = "HHS #"
        ActiveCell.Offset(0, -4).Range("A1").Select
        With Selection.Font
            .Color = -16776961
            .TintAndShade = 0
        End With
        ActiveCell.Offset(2, 0).Range("A1:E1").ClearContents
    End If
    Application.CutCopyMode = False
    Application.ScreenUpdating = True
End Sub

 

[Shiremaid]

Update: I am attempting an rebuild and so far, adding new lines without the macro seems to work but even just doing one worksheet a few times running the macro causes the crashing when doing a save-as to reoccur.
It seems to be ok if I use it to add an Amendment (the ElseIf section) but adding single lines cause problems.
Can running the same macro over and over cause glitches? Adding an Amendment is rare, but adding actions is a very frequent action.
Thanks for any help or advice you can give!

 

[Veritan]

Your code is looking better. I rather doubt that running a script repeatedly would cause glitches. If it did, programmers would never be able to test their code! There's still a couple things that you might want to look at in your code. You have several lines that go something like

ActiveCell.Offset(#,#).Range("...,...").Select
or
ActiveCell.Rows(#,#).Select

I'm rather unsure what these lines are trying to do. I think Excel will only use the Range reference in these lines, but I'm not certain. You should probably remove the ActiveCell.Offset portion and just start the line with Range(...) or Rows(...). If you're trying to select a range of cells with those lines, something like Range("A1", ActiveCell.Offset(2, -1)) is probably a better approach. For example,

    ActiveCell.Offset(2, -1).Range("A1:F1").Select
    Selection.ClearContents

can be replaced with

Range("A1:F1").ClearContents

You also have some lines like

ActiveCell.FormulaR1C1 = "Amendment"

I'm guessing you got that code by recording a macro and then copying what Excel came up with. Excel is nice in that it provides the option to do something like that, and it can help if you need some specific syntax for a particular line of code. However, the catch is that the code it supplies is rarely very efficient or effective. It may get the job done, but it is almost never very dynamic or interactive with other code that you've written. In those lines, remove the FormulaR1C1 component and just make it ActiveCell = "Amendment". If it's easier, you can make it ActiveCell.Value = "Amendment". But since the .Value property is the default property, you aren't required to have it in. The FormulaR1C1 method is used when you want to insert a formula into a cell and that formula has to reference specific cells, especially if those cells can change location or need to have relative, mixed, absolute, or some combination of the above cell references. You don't need it when you're just inserting text into a cell.

See if any of these adjustments help speed up your code. Try to keep removing as many of the Selection methods in there as you can, too. That'll probably help out a lot.

 

[Shiremaid]

I have taken a few more of your suggestions (thank you, and you're right, I did record a lot of this lol) and so far it hasn't glitched at that same line again, so thanks everyone for your help!

I can't remove the ActiveCell bits because most of the code is relative reference type things where I need to activity to happen in different places depending on which cell they double click (there are multiple records that they can add actions/amendments to) unless there's a better way to do relative references?

I am having problems now(again/still) where it crashes completely. Sometimes while I am working and almost anytime I try to do a save as. I have begun to suspect that my major problem might not be the sheet itself but actually my computer or my excel. Is that possible?
While I am the one who uses the workbooks the most, there are other users and none of them have experienced the troubles with it crashing that I have. It might be because I am often doing a lot more at a time, but sometimes it happens within a few actions so maybe there's something wrong with excel or my computer's memory or something? I have the most problems with workbooks when they get larger or I've been working for a while.

Private Sub Worksheet_BeforeDoubleClick(ByVal Target As Range, Cancel As Boolean)
    Application.ScreenUpdating = False
    If Target.Value = "Double-Click to Add An Action" Then
        With ActiveCell.Offset(2)
            .EntireRow.Copy
            .EntireRow.Insert Shift:=xlDown
        End With
        ActiveCell.Offset(2, -1).Range("A1:F1").ClearContents
        ActiveCell.Offset(2, -1).Range("A1").Select
     ElseIf Target.Value = "Double-Click to Add an Amendment" Then
        ActiveCell.Rows("1:3").EntireRow.Select
        Selection.Copy
        Selection.End(xlDown).Select
        ActiveCell.Offset(1, 0).Rows("1:1").EntireRow.Select
        Selection.Insert Shift:=xlDown
        ActiveCell = "Amendment"
        ActiveCell.Offset(0, 4) = "HHS #"
        ActiveCell.Offset(2).Range("A1:E1").ClearContents
        ActiveCell.Offset(2, 0).Range("A1").Select
    End If
    Application.CutCopyMode = False
    Application.ScreenUpdating = True
End Sub

 

[Veritan]

I think I finally understand what you're trying to accomplish. Try using this code instead. A few things to keep in mind: One, it removes the selection method almost completely. It's only used to avoid having the cursor stuck in the middle of a cell after you double click it. Two, it doesn't use the ActiveCell reference, which can potentially slow down the code. Finally, it avoids the ambiguity of using ActiveCell.Offset(#,#).Range(#,#). What Excel was doing in your code was basically counting the number of columns that are in A1:F1 and then applying that size to the selection. I've never actually seen anyone do that before. I'm rather surprised Excel was even able to run code like that, I didn't think it could. But, like I said, VBA is a very forgiving programming language, so there must be some background code that enables it to understand what you're inferring.

If rows aren't being copied to the right location or if the wrong cell is having its value changed, try editing the Offset and/or Resize values before changing anything else. I'm basically guessing what your sheet looks like. I think this will do what you're asking, but the Offset/Resize values may need some tweaking to fit your exact sheet.

Option Explicit
Private Sub Worksheet_BeforeDoubleClick(ByVal Target As Range, Cancel As Boolean)
    Application.ScreenUpdating = False
    If Target = "Double-Click to Add An Action" Then
        With Target.Offset(2)
            .EntireRow.Copy
            .EntireRow.Insert Shift:=xlDown
            .Offset(, -1).Resize(1, 6).ClearContents
            .Offset(, -1).Select
        End With
    ElseIf Target = "Double-Click to Add an Amendment" Then
        Target.Resize(3).EntireRow.Copy
        With Target.End(xlDown)
            .Offset(1).EntireRow.Insert Shift:=xlDown
            .Offset(1) = "Amendment"
            .Offset(1, 4) = "HHS #"
            .Offset(2).Resize(2, 5).ClearContents
            .Offset(2).Select
        End With
    End If
    Application.CutCopyMode = False
    Application.ScreenUpdating = True
End Sub

Let me know if this works for you. If you are able to get this code working so the values are all in the right place and it is still freezing/crashing, then the culprit is almost certainly your computer. See if you're able to get a faster one or a later version of Excel (I'm using Excel 2013, in case you're wondering).

 

[Shiremaid]

Thank you!
Took some tweaking to figure out how to get things where I needed them but I got it!
And I very much appreciate the code, I will have to remember to use "with" in the future and avoid using ActiveCell.

Still crashing on Save As when the workbook's been open for a while, but much less crashing while working with it, so at this point I'm pretty sure it's our computer system (we have weird issues with Outlook and Word sometimes too) and I can just save and close and use copy instead.