Logo 
Search:

MS Office Answers

Ask Question   UnAnswered
Home » Forum » MS Office       RSS Feeds
  Question Asked By: Eden Jones   on Oct 07 In MS Office Category.

  
Question Answered By: Gloria Cook   on Oct 07

There is certainly some basic tidying you should do in and around your IF
statement.

I suspect that your indenting might not be consistent. (Hard to tell in an
e-mail, seeing they tend to get mucked up.) But your re-enabling of the
events and screen updating  is in the "else" part of the IF. It shouldn't be
in the IF at all. So let's move it. I'll put leading full-stops on the
lines to attempt to keep the indenting intact.

. If ThisWorkbook.Sheets("shiftreport").Range("b541") =
DestSheet.Range("b2").Value Then
. Set DestRange = DestSheet.Range("A" & Lr + 2)
. SourceRange.Copy
. DestRange.PasteSpecial Paste:=xlPasteValuesAndNumberFormats,
Operation:=xlNone, SkipBlanks:=yes, Transpose:=False
. Application.CutCopyMode = False
. DestWB.Close savechanges:=True
. Else
. Set DestRange = DestSheet.Range("A" & Lr + 2)
. SourceRange.Copy
. Selection.Insert Shift:=xlDown
. DestRange.PasteSpecial Paste:=xlPasteValuesAndNumberFormats,
Operation:=xlNone, SkipBlanks:=yes, Transpose:=False
. Application.CutCopyMode = False
. DestWB.Close savechanges:=True
. End If
. With Application
. .ScreenUpdating = True
. .EnableEvents = True
. End With

Now look at the true  and false  parts of the IF - they're almost identical.
In fact, the only difference is the "insert" statement. So, move the common
code outside the IF.

. If ThisWorkbook.Sheets("shiftreport").Range("b541") <>
DestSheet.Range("b2").Value Then
. Selection.Insert Shift:=xlDown
. End If
. SourceRange.Copy
. Set DestRange = DestSheet.Range("A" & Lr + 2)
. DestRange.PasteSpecial Paste:=xlPasteValuesAndNumberFormats,
Operation:=xlNone, SkipBlanks:=yes, Transpose:=False
. Application.CutCopyMode = False
. DestWB.Close savechanges:=True

But ... does your code  actually work? I suspect that it doesn't.

Your insert  statement is Selection.Insert. I would have expected the
selection to be in your source sheet, not your destination, which means the
insert will happen in the wrong place. Don't you want DestRange.Insert?

Also, your insert is presumably only inserting one row, but your copy  is
copying 20 rows. This is presumably wrong.

Also, you have a variable "Lr" which is used to control row numbers, but
appears to be left at zero the whole time.

Lastly, a hard-coded range  such as "A541:Q560" - particularly when this is
not the only use of 541 - is definitely not a good idea. Unless you are
absolutely sure that the rest of the sheet will stay as it is, you should
use a named range for this area.

Lastly, you should never use "On error  Resume Next" unless there is a
specific technical reason to do so. If you get run-time errors, they are
usually due to something you are doing wrong. If you need it at all, put it
just before the statement that needs it, then turn error checking back on
immediately after the statement involved.

Share: 

 

This Question has 8 more answer(s). View Complete Question Thread

 
Didn't find what you were looking for? Find more on Copy_To_Another_Workbook Or get search suggestion and latest updates.