Can this VBA code be written better?

[kalim]

Hi excel users.<?xml:namespace prefix = o ns = "urn:schemas-microsoft-comfficeffice" /><o></o>
<o></o>
I was wondering if anyone could look at this VBA code to see if it can be done more efficiently (not good with VBA) Thanks. In case you are wondering "b2" has a value that needs to be cleared first.

Sub list_1()
 
Sheet1.Activate
 
    Range("B2").ClearContents
    Range("f2:f41").Select
    Selection.Copy
    Range("A2").Select
    Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
        :=False, Transpose:=False
        Range("A42:A53").Select
    Selection.ClearContents
    Sheet3.Select
    Range("B1").Select
 
End Sub

 

[GTO]

ACK! and YIKES! If I keep this up, I'll run out of 'not actually dirty' words...

Drop the .Select in the .GoTo. Its un-needed.

@Jack: I agree, just the OP specified that B2 had to be cleared first. Thanks for the new word (and yes, the blonde guy had to look it up to be sure...)

 

[JackDanIce]

Ahh Mark and I didn't notice that I blindly copied your .Select in the Goto!

If B2 has to be cleared first, what about:
.Range("B2, A42:A53").ClearContents

Good spot though, I guess it depends on whether the OP has dependent cells in B2 and F2:F41 respectively and how they're being driven before values are copied into A2:A41

 

[snb_]

As you can see in the OP's code she needs to select sheet1 first.
That means that sheet1 isn't the activesheet.
So I suppose another sheet (and I suspect that it is sheet3) to be the activesheet.
That's why I fear that any activate, select or goto is redundant.

 

[kalim]

Thanks all for the help, you are all great.<?xml:namespace prefix = o ns = "urn:schemas-microsoft-comfficeffice" /><o></o>
<o></o>
I am a little unsure which code would be best for me so I’ll answer your questions.<o></o>
<o></o>
1) With b2 - sorry I have made a mistake with that. It does need to be cleared first, but it already gets cleared with VBA before this macro/VBA code runs, so it is not an issue anymore. So no need to clear it twice.<o></o>
<o></o>
2) Sheet3 is the activesheet - the macro will run from that sheet. Sheet1 is where the data is. However, I still need to add select (sheet3) or similar to the end of the code as it will stay on sheet1 right?<o></o>
<o></o>
3) And sorry by better I meant more efficient/quicker.<o></o>

 

[VoG]

No need to select a sheet. Try

Sub list_1()
With Sheet1
    .Range("f2:f41").Copy
    .Range("A2").PasteSpecial Paste:=xlPasteValues
    .Range("A42:A53").ClearContents
End With
End Sub

 

[kalim]

Hi,
I have just tried your code Colin and have run into a problem. The code runs fine (by itself), but I have it set up so it is run from another macro and for some reason, this breaks it. The problem is, it does not copy the values over from cells F42 onwards. It stops at F41, so all the values beyond that do not get copied.
I am pretty sure the problem lies with the ‘with sheet1’ part of the change_list code. Any suggestions? Code below. And thanks VoG I will have to try your code later as well.

Sub change_list()
With Sheet1
 
    If Range("a1") = "list 1" Then
    list_1
    Else
        If Range("a1") = "list 2" Then
        list_2
        Else
            If Range("a1") = "list 3" Then
            list_3
            Else
                If Range("a1") = "list 4" Then
                list_4
                Else
                    list_5
 
End If
    End If
        End If
            End If
                End With
End Sub

Sub list_1()
 
    With Sheet1.Range("f2:f41")
        Sheet1.Range("A2").Resize(.Rows.Count, .Columns.Count).Value = .Value
    End With
 
    Sheet1.Range("A42:A53").ClearContents
 
End Sub
Sub list_2()
    With Sheet1.Range("f2:f53")
        Sheet1.Range("A2").Resize(.Rows.Count, .Columns.Count).Value = .Value
    End With
 
    Sheet1.Range("A54").ClearContents
 
End Sub
Sub list_3()
    With Sheet1.Range("f2:f42")
        Sheet1.Range("A2").Resize(.Rows.Count, .Columns.Count).Value = .Value
    End With
 
    Sheet1.Range("A43:A53").ClearContents
 
End Sub
Sub list_4()
    With Sheet1.Range("f2:f47")
        Sheet1.Range("A2").Resize(.Rows.Count, .Columns.Count).Value = .Value
    End With
 
    Sheet1.Range("A48:A53").ClearContents
End Sub
Sub list_5()
    With Sheet1.Range("f2:f42")
        Sheet1.Range("A2").Resize(.Rows.Count, .Columns.Count).Value = .Value
    End With
 
    Sheet1.Range("A43:A53").ClearContents
 
End Sub

 

[snb_]

This will suffice:

With Sheet1.Range("f2:f41").resize(40+ choose(replace([A1],"list ",""),0,12,1,6,1))
  .offset(,-5) = .Value
  .offset(40).resize(54-.rows.count).ClearContents
end with

And if you fill A1 with 1,2,3,4 or 5 it can be done using:

With Sheet1.Range("f2:f41").resize(40+ choose([A1],0,12,1,6,1))
  .offset(,-5) = .Value
  .offset(40).resize(54-.rows.count).ClearContents
end with

 

[kalim]

Thanks snb_

I have one problem thou. I get a run time error 13 mismatch when I run it from sheet3 (the active sheet). It stops at the first line. I believe that the [A1] section of that line is the problem. When sheet3 is the active sheet the code searches for list " in a1. It can't find it so it returns an error.

I have a workaround (linking the a1 cells on both sheets), it works fine with that, but is there a way to change the VBA code so I don't have to.

And shouldn't that 'with' sheet1 statement make the VBA code look at A1 on sheet1 not 3?
Thanks code below.

Sub change_list()
  With Sheet1.Range("f2:f41").Resize(40 + Choose(Replace([A1], "list ", ""), 0, 12, 1, 6, 1))
    .offset(,-5).resize(52).clearcontents
    .Offset(, -5) = .Value
  End With
End Sub

 

[snb_]

I think it's evident where to change the code if you want it to run in sheet3:

Sub change_list()
  With Sheet1.Range("f2:f41").Resize(40 + Choose(Replace([sheet1!A1], "list ", ""), 0, 12, 1, 6, 1))
    .offset(,-5).resize(52).clearcontents
    .Offset(, -5) = .Value
  End With
End Sub

 

[Boller]

Using the Range property is better than using Evaluate []. It benefits from early binding, thus you get intellisense and it is more efficient. It also (IMO) makes the code clearer to read.

I disagree - I prefer to use Evaluate[].

What intellisense do you get using the Range prpoerty that you do not get using Evaluate[] ?

I know all the theory about it being more efficient but have never a seen a measurable demonstration of how much more efficient.
Can you suggest one?

IMO using Evaluate[] makes the code clearer to read (your opposite opinion is probably because you are used to writing/reading code using the range property).
Evaluate[] also has the advantage of being quicker to type.