This is not working because I didn't write it correctly

L

L.White

Unfortunatly, I am also not experienced enough to figure out what I have
done wrong. Please take a look at this horrible code and tell me what I have
done wrong.

The point of this is to automatically update information when the workbook
is opened. This is a time off tracking sheet. there is a master blank copy
used for creating new sheets. A census form that is a repository for other
general informatoin for all employees. Vac&Sick sheet is an index similiar
to the census form. Each remaining sheet is the individuals record fo time
taken off.

Thanks for helping and if you have any questions let me know. By the way,
this started with another thread but I have lost it somehow. For whatever
reason I am unable to access my old thread. Sorry for any confusion or
reposting that has occured.
LWhite

Sub Workbook_Open()
Dim wks As Worksheet
For Each wks In ActiveWorkbook.wks
' pick an employee record to evaluate rather than the master and system
sheets
'
If LCase(wks.Name) = LCase("master") Then
' do nothing
Else
If LCase(wks.Name) = LCase("employee census") Then
' do nothing
Else
If LCase(wks.Name) = LCase("vac&sick") Then
' do nothing
Else

' Determine if the employee uses the first of the year or not

If ActiveSheet.Range("C5").Value > DateSerial(2005, 1, 1) Then
' if they do then do the copy/move
If Range("A99").Value > 0 Then
Range("A7:F34").Select
Selection.Copy
ActiveWindow.SmallScroll Down:=66
Range("A100").Select
ActiveSheet.Paste
ActiveWindow.SmallScroll Down:=-75
Range("A8:F34").Select
Application.CutCopyMode = False
Selection.ClearContents
Range("A99").Value = 1
End If
Else
' if not then determine if the copy should be done or not
If ActiveSheet.Range("C5").Value > DateSerial(Now - 1, Range("C5"),
Range("C5")) Then
If Range("A99").Value > 0 Then
Range("A7:F34").Select
Selection.Copy
ActiveWindow.SmallScroll Down:=66
Range("A100").Select
ActiveSheet.Paste
ActiveWindow.SmallScroll Down:=-75
Range("A8:F34").Select
Application.CutCopyMode = False
Selection.ClearContents
Range("A99").Value = 1
End If
End If
End If
End If
End If
End If

Next wks
End Sub
 
B

Bob Phillips

What exactly is the problem?

--

HTH

RP
(remove nothere from the email address if mailing direct)
 
L

L.White

Sorry about that.

To test this I changed the date in my machine to be past the first of next
year. If this were written correctly all people hired before 2005 would have
had the days off moved to a lower position on their sheet and the main usage
area blanked. Nothing happened at all.

LWhite
 
B

Bob Phillips

Maybe it is because you have fully qualified all objects. Try this

Sub Workbook_Open()
Dim wks As Worksheet
For Each wks In ActiveWorkbook.wks
' pick an employee record to evaluate rather than the master and system
Sheets
With wks
If LCase(.Name) = "master" Or _
LCase(.Name) = "employee census" Or _
LCase(.Name) = "vac&sick" Then
' do nothing
ElseIf .Range("C5").Value > DateSerial(2005, 1, 1) Or _
.Range("C5").Value > DateSerial(Now - 1, .Range("C5"),
..Range("C5")) Then
' Determine if the employee uses the first of the year or
' if not then determine if the copy should be done or not
If .Range("A99").Value > 0 Then
.Range("A7:F34").Copy .Range("A100")
.Range("A8:F34").ClearContents
.Range("A99").Value = 1
End If
End If
End With
Next wks
End Sub

--

HTH

RP
(remove nothere from the email address if mailing direct)
 
B

Bob Phillips

That should be NOT fully qualified.

--

HTH

RP
(remove nothere from the email address if mailing direct)
 
L

L.White

I am receiving a compiler error of End With without With. If I comment out
the End With I receive the same error saying Next without For Each. I don't
get that since both statements are clearly there. Here is the code I have in
place now.


Sub Workbook_Open()
Dim wks As Worksheet
For Each wks In ActiveWorkbook.wks
' pick an employee record to evaluate rather than the master and system
Sheets
With wks
If LCase(.Name) = "master" Or _
LCase(.Name) = "employee census" Or _
LCase(.Name) = "vac&sick" Then
' do nothing
Else
If .Range("C5").Value > DateSerial(2005, 1, 1) Or _
.Range("C5").Value > DateSerial(Now - 1, .Range("C5"),
..Range("C5")) Then
' Determine if the employee uses the first of the year or
' not then determine if the copy should be done or not
If .Range("A99").Value = 0 Then
.Range("A7:F34").Copy .Range("A100")
.Range("A7:F34").ClearContents
.Range("A99").Value = 1
End If
End If
End With
Next wks
End Sub

By the way Bob, I like the way that this code looks a lot better than what I
had before. This is much cleaner and easier to read. Thanks for helping.
Leonard
 
L

L.White

Oh yeah, the code I posted a moment ago is in the ThisWorkbook section of
the spreadsheet. Is that the correct location?

Leonard
 
I

impslayer

L.White skrev:
I am receiving a compiler error of End With without With. If I comment out
the End With I receive the same error saying Next without For Each. I don't
get that since both statements are clearly there. Here is the code I have in
place now.


Sub Workbook_Open()
Dim wks As Worksheet
For Each wks In ActiveWorkbook.wks
' pick an employee record to evaluate rather than the master and system
Sheets
With wks
If LCase(.Name) = "master" Or _
LCase(.Name) = "employee census" Or _
LCase(.Name) = "vac&sick" Then
' do nothing
Else
If .Range("C5").Value > DateSerial(2005, 1, 1) Or _
.Range("C5").Value > DateSerial(Now - 1, .Range("C5"),
.Range("C5")) Then
' Determine if the employee uses the first of the year or
' not then determine if the copy should be done or not
If .Range("A99").Value = 0 Then
.Range("A7:F34").Copy .Range("A100")
.Range("A7:F34").ClearContents
.Range("A99").Value = 1
End If
End If
End With
Next wks
End Sub

By the way Bob, I like the way that this code looks a lot better than what I
had before. This is much cleaner and easier to read. Thanks for helping.
Leonard

Might be better to add an 'End If'.

/impslayer, aka Birger Johansson
 
B

Bob Phillips

Leonard,

It may have been the wrap-around causing it.

I have looked at the code again and there are a couple of other problems,
hopefully all corrected here

Sub Workbook_Open()
Dim wks As Worksheet
For Each wks In ActiveWorkbook.Worksheets
'pick employee record to evaluate rather than master and system Sheets
With wks
If LCase(.Name) = "master" Or _
LCase(.Name) = "employee census" Or _
LCase(.Name) = "vac&sick" Then
' do nothing
ElseIf .Range("C5").Value > DateSerial(2005, 1, 1) Or _
.Range("C5").Value > Now - 1 Then
' Determine if the employee uses the first of the year or
' if not then determine if the copy should be done or not
If .Range("A99").Value > 0 Then
.Range("A7:F34").Copy .Range("A100")
.Range("A8:F34").ClearContents
.Range("A99").Value = 1
End If
End If
End With
Next wks
End Sub


and yes, it does go in ThisWorkbook.

--

HTH

RP
(remove nothere from the email address if mailing direct)
 
L

L.White

This still is not working. I tested things by doing all of the following.

1. Copy the formula in the workbook.
2. Place a 0 in every A99 cell for a sheet that should have this happen.
3. Save and close.
4. Reset the windows clock to think that the date is January 17, 2006.
5. Reopen the workbook and wait for the system to process everything that it
is going to.
6. Cross fingers.
7. Check a sheet to see if the copy and paste has occured.
8. Find that it has not and curse.

Here is the code that I have in place.

Sub Workbook_Open()
Dim wks As Worksheet
For Each wks In ActiveWorkbook.Worksheets
'pick employee record to evaluate rather than master and system Sheets
With wks
If LCase(.Name) = "master" Or _
LCase(.Name) = "employee census" Or _
LCase(.Name) = "vac&sick" Then
' do nothing
ElseIf .Range("C5").Value < DateSerial(2005, 1, 1) Or _
.Range("C5").Value < Now - 1 Then
' Determine if the employee uses the first of the year or
' if not then determine if the copy should be done or not
If .Range("A99").Value = 0 Then
.Range("A7:F34").Copy .Range("A100")
.Range("A7:F34").ClearContents
.Range("A99").Value = 1
End If
End If
End With
Next wks
End Sub

Note that the value in C5 should be less than the value of 01/01/2005 to
trigger the first condition.
Note that if the value of A99 is 1 then nothing should happen since the last
action sets A99 to 1.

This does not seem to be doing anything on open. Is Sub Workbook_Open() the
right command? Should this be a private sub instead?

Leonard
 
B

Bob Phillips

Hi Leonard,

Good to see you are still laughing about it :)).

Seeing that resume of what should happen, I don't think the code does that.
I have documented inline what I think it does do
1. Copy the formula in the workbook.
2. Place a 0 in every A99 cell for a sheet that should have this happen.
3. Save and close.
4. Reset the windows clock to think that the date is January 17, 2006.

I don't know what formula you are referring to, so I don't see the relevance
of that bit.

What do you mean by every A99, there is only one on a worksheet? Do you per
worksheet?

Did you store the code in the ThisWorkbook code module? it should not be in
a normal code module.
5. Reopen the workbook and wait for the system to process everything that it
is going to.
6. Cross fingers.
7. Check a sheet to see if the copy and paste has occured.

See code comments
8. Find that it has not and curse.

Here is the code that I have in place.

Sub Workbook_Open()
Dim wks As Worksheet
For Each wks In ActiveWorkbook.Worksheets

Loop through each worksheet in the workbook
'pick employee record to evaluate rather than master and system Sheets
With wks
If LCase(.Name) = "master" Or _
LCase(.Name) = "employee census" Or _
LCase(.Name) = "vac&sick" Then
' do nothing

It ignores the three sheets calle master, employee census, and vac&sick
ElseIf .Range("C5").Value < DateSerial(2005, 1, 1) Or _
.Range("C5").Value < Now - 1 Then

This checks whether the date in C5 (on the current sheet being processed) is
less than Jan 1 2005 (NOTE 2005 not 2006), of earlier thatn yesterday (which
would be 12th Dec 2005 after you set your clock). This is probably correct?
' Determine if the employee uses the first of the year or
' if not then determine if the copy should be done or not
If .Range("A99").Value = 0 Then
.Range("A7:F34").Copy .Range("A100")
.Range("A7:F34").ClearContents
.Range("A99").Value = 1

If the value in A99 (on the current sheet being processed) is 0, then copy
A7:A34 to A100, clear A7:A34, set A99 to 1
End If
End If
End With
Next wks
End Sub

This seems OK(ish?), probably just not stored where it should be.

You could test it without changing the system clock on one sheet to prior to
1st Jan 2005, and run it.

Fingers crossed

Bob
 
L

L.White

I just hope someone is laughing.

When I open the workbook nothing happens. So I go into the VBa Editor.

When I go to the code and click on run in the ThisWorkbook location I place
the cursor at the end of the first line. The code worked for the test
sheets. The only problem I am now having is getting this to run when the
workbook is opened. Any other ideas?

To confirm:
1. The code is in the ThisWorkBook object.
2. When I said every A99 I did mean the A99 on each sheet of the workbook.
Sorry for the confusion.

I found that the Now -1 is incorrect. To check for a date being a one year
anniversary I am checking Now - 365.

Just for the fun of it. here is the whole bit of code again.

Sub Workbook_Open()
Dim wks As Worksheet
For Each wks In ActiveWorkbook.Worksheets
'pick employee record to evaluate rather than master and system Sheets
With wks
If LCase(.Name) = "master" Or _
LCase(.Name) = "employee census" Or _
LCase(.Name) = "vac&sick" Then
' do nothing
ElseIf .Range("C5").Value < DateSerial(2005, 1, 1) Or _
.Range("C5").Value < Now - 365 Then
' Determine if the employee uses the first of the year or
' if not then determine if the copy should be done or not
If .Range("A99").Value = 0 Then
.Range("A7:F34").Copy .Range("A100")
.Range("A8:F34").ClearContents
.Range("A99").Value = 1
End If
End If
End With
Next wks
End Sub

Leonard
 
B

Bob Phillips

How about mailing me a workbook?

--

HTH

RP
(remove nothere from the email address if mailing direct)
 
L

L.White

I tried responding to this thread earlier but nobody answered. I am trying
again before restarting the thread.

I want the following code to automatically execute when file opens. Right
now the code is located in the ThisWorkbook location. Why isn't it running
on open?

LWhite

Sub Workbook_Open()
Dim wks As Worksheet
For Each wks In ActiveWorkbook.Worksheets
'pick employee record to evaluate rather than master and system Sheets
With wks
If LCase(.Name) = "master" Or _
LCase(.Name) = "employee census" Or _
LCase(.Name) = "vac&sick" Then
' do nothing
ElseIf .Range("C5").Value < DateSerial(2005, 1, 1) Or _
.Range("C5").Value < Now - 365 Then
' Determine if the employee uses the first of the year or
' if not then determine if the copy should be done or not
If .Range("A99").Value = 0 Then
.Range("A7:F34").Copy .Range("A100")
.Range("A8:F34").ClearContents
.Range("A99").Value = 1
End If
End If
End With
Next wks
End Sub
 
B

Bob Phillips

I responded suggesting you mail it to me.

--

HTH

RP
(remove nothere from the email address if mailing direct)
 

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

Top