Excel problem - why won't my macro work?

  • Thread starter Thread starter madbloke
  • Start date Start date
M

madbloke

I am a beginner at these things, so hopefully it's a simple error I've
made.

I have an excel workbook. On the first page is a master list of
documents, with column G containing a letter denoting the status of the
document (C for current, D for deleted etc.)

I regularly need to issue people with up to date lists of current and
deleted documents, so I wrote this macro to copy the relevant entries
onto seperate worksheets. At the moment it's only looking for Current
entries, but it just won't work.

Anyone have any idea why not? Please tell me it's simple....



Private Sub Worksheet_Activate()

Dim num As String

For i = 2 To 150

num = i

If Worksheets("MASTER LIST").Cells(i, 5) = "C" Then

Range("A" & num:"G" & num).Select

Selection.copy

Sheets("CURRENT ").Select

Range("A" & num).Select

ActiveSheet.Paste

Else

'messagebox is only here for my testing purposes, and will be
'deleted once the macro is working

MsgBox ("Not current")

End If

Next i

End Sub
 
Change
Range("A" & num:"G" & num).Select
to
Sheets("MASTER LIST").Activate
Range("A" & num & ":G" & num).Select

Note that
If Worksheets("MASTER LIST").Cells(i, 5) = "C" Then
looks in column E, not column G (column 7).

Also, does your Current sheet really have a space following the sheet name?
Sheets("CURRENT ").Select
(I would use .Acivate instead of .Select here, but either will work)

Jerry
 
hi,
i try to run your code in one of my sheets and ran into a
number of issues so i just re-vamped the whole thing
try this
Private Sub copycurrent()

Worksheets("master list").Select
Dim num As Range
Dim num2 As Range
Dim numb As Range
Dim num2b As Range
Set num = Cells(2, 1)
Set num2 = Sheets("current").Cells(1, 1)
Do While Not IsEmpty(num)
Set numb = num.Offset(1, 0)
Set num2b = num2.Offset(1, 0)
If num.Value = "C" Then
Range(num, num.Offset(0, 7)).Copy
Sheets("current").Select
ActiveSheet.Paste
Sheets("master list").Select
Set num = numb
Set num2 = num2b
End If
Set num = numb
Loop

End Sub
 
Thanks for that. It sort-of works (i.e. it doesnt give me an erro
anymore).

Unfortunately, it doesn't actually copy any info over to the 'Current
sheet (although it looks like it's trying to.)

It's actually column E that has the marker, so that was my mistake. An
the Current sheet _did_ have a space, which I've now deleted (an
changed the macro accordingly.)

Any ideas how to make it actually copy the data
 
When you select the other sheet, you never come back. You also had an error
in your address string. You have the name
Sheets("CURRENT ")
with a space on the end. I took out the space, but if there really is a
space in the name, you need to add it back in.

Private Sub Worksheet_Activate()
Dim num As String
For i = 2 To 150
num = i
If Worksheets("MASTER LIST").Cells(i, 5) = "C" Then
Worksheets("MASTER LIST").Range("A" & _
num & ":G" & num).copy Destination:= _
Sheets("CURRENT").Range("A" & num)
Else
'messagebox is only here for my testing purposes, and will be
'deleted once the macro is working
MsgBox ("Not current")
End If
Next i
End Sub
 
Anonymous,

Dunno if it's something i'm doing wrong, but your code doesn't seem t
do anything when I add it to my sheet?
hi,
i try to run your code in one of my sheets and ran into a
number of issues so i just re-vamped the whole thing
try this
Private Sub copycurrent()

Worksheets("master list").Select
Dim num As Range
Dim num2 As Range
Dim numb As Range
Dim num2b As Range
Set num = Cells(2, 1)
Set num2 = Sheets("current").Cells(1, 1)
Do While Not IsEmpty(num)
Set numb = num.Offset(1, 0)
Set num2b = num2.Offset(1, 0)
If num.Value = "C" Then
Range(num, num.Offset(0, 7)).Copy
Sheets("current").Select
ActiveSheet.Paste
Sheets("master list").Select
Set num = numb
Set num2 = num2b
End If
Set num = numb
Loop

End Sub
 
Cheers! That works!

Couple of little issues that you might be able to help me wit
(probably a bit beyond my capabilities!)

As it is now, the macro leaves blank rows for non-compliant entries. I
there any way to get round this?

Also, I'd need it to refresh each time the Current sheet is selected
(I.E. I'd need it to clear the sheet, and pull the info through again
otherwise changes to the master list don't show up). Is there a simpl
command that i'd put before the If statement?


Tom said:
When you select the other sheet, you never come back. You also ha
an error
in your address string. You have the name
Sheets("CURRENT ")
with a space on the end. I took out the space, but if there reall
is a
space in the name, you need to add it back in.

Private Sub Worksheet_Activate()
Dim num As String
For i = 2 To 150
num = i
If Worksheets("MASTER LIST").Cells(i, 5) = "C" Then
Worksheets("MASTER LIST").Range("A" & _
num & ":G" & num).copy Destination:= _
Sheets("CURRENT").Range("A" & num)
Else
'messagebox is only here for my testing purposes, and will be
'deleted once the macro is working
MsgBox ("Not current")
End If
Next i
End Sub
 
Well Anonymous's code is checking column A for the Value C rather than
checking column E. If you use it in the activate event of either of the
sheets it should go into a recursive whirlwind. (untested)

--
Regards,
Tom Ogilvy

madbloke > said:
Anonymous,

Dunno if it's something i'm doing wrong, but your code doesn't seem to
do anything when I add it to my sheet?
hi,
i try to run your code in one of my sheets and ran into a
number of issues so i just re-vamped the whole thing
try this
Private Sub copycurrent()

Worksheets("master list").Select
Dim num As Range
Dim num2 As Range
Dim numb As Range
Dim num2b As Range
Set num = Cells(2, 1)
Set num2 = Sheets("current").Cells(1, 1)
Do While Not IsEmpty(num)
Set numb = num.Offset(1, 0)
Set num2b = num2.Offset(1, 0)
If num.Value = "C" Then
Range(num, num.Offset(0, 7)).Copy
Sheets("current").Select
ActiveSheet.Paste
Sheets("master list").Select
Set num = numb
Set num2 = num2b
End If
Set num = numb
Loop

End Sub
 
You show the Worksheet_Activate event, so I assume this is in the code
module for the "CURRENT" sheet.

Private Sub Worksheet_Activate()
Dim num As String
Me.UsedRange.ClearContents
Num = 1
For i = 2 To 150

If Worksheets("MASTER LIST").Cells(i, 5) = "C" Then
Worksheets("MASTER LIST").Range("A" & _
i & ":G" & i).copy Destination:= _
Sheets("CURRENT").Range("A" & num)
num = num + 1
Else
'messagebox is only here for my testing purposes, and will be
'deleted once the macro is working
'MsgBox ("Not current")
End If
Next i
End Sub

this should update the data each time CURRENT is activated. (that is one
reason you don't want to use Select and Activate for sheets within the
code).

--
Regards,
Tom Ogilvy


madbloke > said:
Cheers! That works!

Couple of little issues that you might be able to help me with
(probably a bit beyond my capabilities!)

As it is now, the macro leaves blank rows for non-compliant entries. Is
there any way to get round this?

Also, I'd need it to refresh each time the Current sheet is selected.
(I.E. I'd need it to clear the sheet, and pull the info through again,
otherwise changes to the master list don't show up). Is there a simple
command that i'd put before the If statement?


Tom said:
When you select the other sheet, you never come back. You also had
an error
in your address string. You have the name
Sheets("CURRENT ")
with a space on the end. I took out the space, but if there really
is a
space in the name, you need to add it back in.

Private Sub Worksheet_Activate()
Dim num As String
For i = 2 To 150
num = i
If Worksheets("MASTER LIST").Cells(i, 5) = "C" Then
Worksheets("MASTER LIST").Range("A" & _
num & ":G" & num).copy Destination:= _
Sheets("CURRENT").Range("A" & num)
Else
'messagebox is only here for my testing purposes, and will be
'deleted once the macro is working
MsgBox ("Not current")
End If
Next i
End Sub

 
That's working perfectly! Thanks!

And the good bit is that I pretty much understand why! Learning i
fun!!

Tom said:
You show the Worksheet_Activate event, so I assume this is in th
code
module for the "CURRENT" sheet.

Private Sub Worksheet_Activate()
Dim num As String
Me.UsedRange.ClearContents
Num = 1
For i = 2 To 150

If Worksheets("MASTER LIST").Cells(i, 5) = "C" Then
Worksheets("MASTER LIST").Range("A" & _
i & ":G" & i).copy Destination:= _
Sheets("CURRENT").Range("A" & num)
num = num + 1
Else
'messagebox is only here for my testing purposes, and will be
'deleted once the macro is working
'MsgBox ("Not current")
End If
Next i
End Sub

this should update the data each time CURRENT is activated. (that i
one
reason you don't want to use Select and Activate for sheets withi
the
code).
 
madbloke said:
Cheers! That works!

Couple of little issues that you might be able to help me with
(probably a bit beyond my capabilities!)

As it is now, the macro leaves blank rows for non-compliant entries. Is
there any way to get round this?

Also, I'd need it to refresh each time the Current sheet is selected.
(I.E. I'd need it to clear the sheet, and pull the info through again,
otherwise changes to the master list don't show up). Is there a simple
command that i'd put before the If statement?


Tom said:
When you select the other sheet, you never come back. You also had
an error
in your address string. You have the name
Sheets("CURRENT ")
with a space on the end. I took out the space, but if there really
is a
space in the name, you need to add it back in.

Private Sub Worksheet_Activate()
Dim num As String
For i = 2 To 150
num = i
If Worksheets("MASTER LIST").Cells(i, 5) = "C" Then
Worksheets("MASTER LIST").Range("A" & _
num & ":G" & num).copy Destination:= _
Sheets("CURRENT").Range("A" & num)
Else
'messagebox is only here for my testing purposes, and will be
'deleted once the macro is working
MsgBox ("Not current")
End If
Next i
End Sub


This is a test
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Back
Top