Improve macro help

I have the following code that works fine and does what I need it to do. What
I am wondering is if there is a way to improve the code inside the if
statement so that the 2 workbooks do not need to be alternately selected.
Thus I hope speed up the macro.

The bit inside takes a range of 120 cells in one row from "FromWorkbook"
then selects "ToWorkbook" and the first cell where the range is to be pasted
and then pastes the data.

If Workbooks(ToWorkbook).Sheets("LTDIS pay").Cells(j, 1).Value =
Workbooks(FromWorkbook).Sheets(1).Cells(i, 3).Value, 2) Then

Workbooks(FromWorkbook).Sheets(1).Activate
Range(Cells(i, 7), Cells(i, 127)).Copy
Workbooks(ToWorkbook).Sheets("LTDIS pay").Activate
Cells(j, 2).Select
ActiveSheet.Paste

End If

Many thanks in advance.

(1) no need to activate the sheets
(2) no need to copy and paste

Workbooks(ToWorkbook).Sheets("LTDIS pay").range(Cells(j, 2),Cells(j,
122)).value=Workbooks(FromWorkbook).Sheets(1).Range(Cells(i, 7),
Cells(i, 127)).value

should do it in the one line for you


fullers wrote:
> I have the following code that works fine and does what I need it to do. What
> I am wondering is if there is a way to improve the code inside the if
> statement so that the 2 workbooks do not need to be alternately selected.
> Thus I hope speed up the macro.
>
> The bit inside takes a range of 120 cells in one row from "FromWorkbook"
> then selects "ToWorkbook" and the first cell where the range is to be pasted
> and then pastes the data.
>
> If Workbooks(ToWorkbook).Sheets("LTDIS pay").Cells(j, 1).Value =
> Workbooks(FromWorkbook).Sheets(1).Cells(i, 3).Value, 2) Then
>
> Workbooks(FromWorkbook).Sheets(1).Activate
> Range(Cells(i, 7), Cells(i, 127)).Copy
> Workbooks(ToWorkbook).Sheets("LTDIS pay").Activate
> Cells(j, 2).Select
> ActiveSheet.Paste
>
> End If
>
> Many thanks in advance.

I replaced the code with your new below but I get the follwing error when it
runs:

Run-time error '1004'

Application-defined or object-defined error

Any ideas as to why the code wont work?

Thanks

"[email protected]" wrote:

> (1) no need to activate the sheets
> (2) no need to copy and paste
>
> Workbooks(ToWorkbook).Sheets("LTDIS pay").range(Cells(j, 2),Cells(j,
> 122)).value=Workbooks(FromWorkbook).Sheets(1).Range(Cells(i, 7),
> Cells(i, 127)).value
>
> should do it in the one line for you
>
>
> fullers wrote:
> > I have the following code that works fine and does what I need it to do. What
> > I am wondering is if there is a way to improve the code inside the if
> > statement so that the 2 workbooks do not need to be alternately selected.
> > Thus I hope speed up the macro.
> >
> > The bit inside takes a range of 120 cells in one row from "FromWorkbook"
> > then selects "ToWorkbook" and the first cell where the range is to be pasted
> > and then pastes the data.
> >
> > If Workbooks(ToWorkbook).Sheets("LTDIS pay").Cells(j, 1).Value =
> > Workbooks(FromWorkbook).Sheets(1).Cells(i, 3).Value, 2) Then
> >
> > Workbooks(FromWorkbook).Sheets(1).Activate
> > Range(Cells(i, 7), Cells(i, 127)).Copy
> > Workbooks(ToWorkbook).Sheets("LTDIS pay").Activate
> > Cells(j, 2).Select
> > ActiveSheet.Paste
> >
> > End If
> >
> > Many thanks in advance.
>
>

if the original worked then mine should work - but to explain it

range("A1").value=1

would set the value of cell A1 on the active sheet to 1

range("a1").value=range("b1").value

would set a1 to the value of b1

extending this therefore means we can reference other workbooks and
other worksheets - HOWEVER, they will need to be open before the code
runs, and the range areas would need to be the same size

fullers wrote:
> I replaced the code with your new below but I get the follwing error when it
> runs:
>
> Run-time error '1004'
>
> Application-defined or object-defined error
>
> Any ideas as to why the code wont work?
>
> Thanks
>
> "[email protected]" wrote:
>
> > (1) no need to activate the sheets
> > (2) no need to copy and paste
> >
> > Workbooks(ToWorkbook).Sheets("LTDIS pay").range(Cells(j, 2),Cells(j,
> > 122)).value=Workbooks(FromWorkbook).Sheets(1).Range(Cells(i, 7),
> > Cells(i, 127)).value
> >
> > should do it in the one line for you
> >
> >
> > fullers wrote:
> > > I have the following code that works fine and does what I need it to do. What
> > > I am wondering is if there is a way to improve the code inside the if
> > > statement so that the 2 workbooks do not need to be alternately selected.
> > > Thus I hope speed up the macro.
> > >
> > > The bit inside takes a range of 120 cells in one row from "FromWorkbook"
> > > then selects "ToWorkbook" and the first cell where the range is to be pasted
> > > and then pastes the data.
> > >
> > > If Workbooks(ToWorkbook).Sheets("LTDIS pay").Cells(j, 1).Value =
> > > Workbooks(FromWorkbook).Sheets(1).Cells(i, 3).Value, 2) Then
> > >
> > > Workbooks(FromWorkbook).Sheets(1).Activate
> > > Range(Cells(i, 7), Cells(i, 127)).Copy
> > > Workbooks(ToWorkbook).Sheets("LTDIS pay").Activate
> > > Cells(j, 2).Select
> > > ActiveSheet.Paste
> > >
> > > End If
> > >
> > > Many thanks in advance.
> >
> >

I removed the range and just copied an indivdual cell over and it worked fine.

Workbooks(ToWorkbook).Sheets("LTDIS pay").Cells(j, 2).Value =
Workbooks(FromWorkbook).Sheets(1).Cells(i, 7).Value

When I put the range back in I get the same error message


"[email protected]" wrote:

> if the original worked then mine should work - but to explain it
>
> range("A1").value=1
>
> would set the value of cell A1 on the active sheet to 1
>
> range("a1").value=range("b1").value
>
> would set a1 to the value of b1
>
> extending this therefore means we can reference other workbooks and
> other worksheets - HOWEVER, they will need to be open before the code
> runs, and the range areas would need to be the same size
>
> fullers wrote:
> > I replaced the code with your new below but I get the follwing error when it
> > runs:
> >
> > Run-time error '1004'
> >
> > Application-defined or object-defined error
> >
> > Any ideas as to why the code wont work?
> >
> > Thanks
> >
> > "[email protected]" wrote:
> >
> > > (1) no need to activate the sheets
> > > (2) no need to copy and paste
> > >
> > > Workbooks(ToWorkbook).Sheets("LTDIS pay").range(Cells(j, 2),Cells(j,
> > > 122)).value=Workbooks(FromWorkbook).Sheets(1).Range(Cells(i, 7),
> > > Cells(i, 127)).value
> > >
> > > should do it in the one line for you
> > >
> > >
> > > fullers wrote:
> > > > I have the following code that works fine and does what I need it to do. What
> > > > I am wondering is if there is a way to improve the code inside the if
> > > > statement so that the 2 workbooks do not need to be alternately selected.
> > > > Thus I hope speed up the macro.
> > > >
> > > > The bit inside takes a range of 120 cells in one row from "FromWorkbook"
> > > > then selects "ToWorkbook" and the first cell where the range is to be pasted
> > > > and then pastes the data.
> > > >
> > > > If Workbooks(ToWorkbook).Sheets("LTDIS pay").Cells(j, 1).Value =
> > > > Workbooks(FromWorkbook).Sheets(1).Cells(i, 3).Value, 2) Then
> > > >
> > > > Workbooks(FromWorkbook).Sheets(1).Activate
> > > > Range(Cells(i, 7), Cells(i, 127)).Copy
> > > > Workbooks(ToWorkbook).Sheets("LTDIS pay").Activate
> > > > Cells(j, 2).Select
> > > > ActiveSheet.Paste
> > > >
> > > > End If
> > > >
> > > > Many thanks in advance.
> > >
> > >
>
>

fullers wrote:
> I removed the range and just copied an indivdual cell over and it worked fine.
>
> Workbooks(ToWorkbook).Sheets("LTDIS pay").Cells(j, 2).Value =
> Workbooks(FromWorkbook).Sheets(1).Cells(i, 7).Value
>
> When I put the range back in I get the same error message
>
>
> "[email protected]" wrote:
>
> > if the original worked then mine should work - but to explain it
> >
> > range("A1").value=1
> >
> > would set the value of cell A1 on the active sheet to 1
> >
> > range("a1").value=range("b1").value
> >
> > would set a1 to the value of b1
> >
> > extending this therefore means we can reference other workbooks and
> > other worksheets - HOWEVER, they will need to be open before the code
> > runs, and the range areas would need to be the same size
> >
> > fullers wrote:
> > > I replaced the code with your new below but I get the follwing error when it
> > > runs:
> > >
> > > Run-time error '1004'
> > >
> > > Application-defined or object-defined error
> > >
> > > Any ideas as to why the code wont work?
> > >
> > > Thanks
> > >
> > > "[email protected]" wrote:
> > >
> > > > (1) no need to activate the sheets
> > > > (2) no need to copy and paste
> > > >
> > > > Workbooks(ToWorkbook).Sheets("LTDIS pay").range(Cells(j, 2),Cells(j,
> > > > 122)).value=Workbooks(FromWorkbook).Sheets(1).Range(Cells(i, 7),
> > > > Cells(i, 127)).value
> > > >
> > > > should do it in the one line for you
> > > >
> > > >
> > > > fullers wrote:
> > > > > I have the following code that works fine and does what I need it to do. What
> > > > > I am wondering is if there is a way to improve the code inside the if
> > > > > statement so that the 2 workbooks do not need to be alternately selected.
> > > > > Thus I hope speed up the macro.
> > > > >
> > > > > The bit inside takes a range of 120 cells in one row from "FromWorkbook"
> > > > > then selects "ToWorkbook" and the first cell where the range is to be pasted
> > > > > and then pastes the data.
> > > > >
> > > > > If Workbooks(ToWorkbook).Sheets("LTDIS pay").Cells(j, 1).Value =
> > > > > Workbooks(FromWorkbook).Sheets(1).Cells(i, 3).Value, 2) Then
> > > > >
> > > > > Workbooks(FromWorkbook).Sheets(1).Activate
> > > > > Range(Cells(i, 7), Cells(i, 127)).Copy
> > > > > Workbooks(ToWorkbook).Sheets("LTDIS pay").Activate
> > > > > Cells(j, 2).Select
> > > > > ActiveSheet.Paste
> > > > >
> > > > > End If
> > > > >
> > > > > Many thanks in advance.
> > > >
> > > >
> >
> >

I think it is a lack of fully qualified ranges

Dim oToWorkbook As Workbook, oFromWorkbook As Workbook
Dim oToSheet As Worksheet, oFromSheet As Worksheet

Set oToWorkbook = Workbooks(ToWorkbook)
Set oFromWorkbook = Workbooks(FromWorkbook)
Set oToSheet = oToWorkbook.Sheets("LTDIS pay")
Set oFromSheet = oFromWorkbook.Sheets(1)

oToSheet.Range(oToSheet.Cells(j, 2), oToSheet.Cells(j, 122)).Value = _
oFromSheet.Range(oFromSheet.Cells(i, 7), oFromSheet.Cells(i, 127)).Value

--
HTH

Bob Phillips

(replace somewhere in email address with gmail if mailing direct)

"fullers" <[email protected]> wrote in message
news:[email protected]...
> I removed the range and just copied an indivdual cell over and it worked
fine.
>
> Workbooks(ToWorkbook).Sheets("LTDIS pay").Cells(j, 2).Value =
> Workbooks(FromWorkbook).Sheets(1).Cells(i, 7).Value
>
> When I put the range back in I get the same error message
>
>
> "[email protected]" wrote:
>
> > if the original worked then mine should work - but to explain it
> >
> > range("A1").value=1
> >
> > would set the value of cell A1 on the active sheet to 1
> >
> > range("a1").value=range("b1").value
> >
> > would set a1 to the value of b1
> >
> > extending this therefore means we can reference other workbooks and
> > other worksheets - HOWEVER, they will need to be open before the code
> > runs, and the range areas would need to be the same size
> >
> > fullers wrote:
> > > I replaced the code with your new below but I get the follwing error
when it
> > > runs:
> > >
> > > Run-time error '1004'
> > >
> > > Application-defined or object-defined error
> > >
> > > Any ideas as to why the code wont work?
> > >
> > > Thanks
> > >
> > > "[email protected]" wrote:
> > >
> > > > (1) no need to activate the sheets
> > > > (2) no need to copy and paste
> > > >
> > > > Workbooks(ToWorkbook).Sheets("LTDIS pay").range(Cells(j, 2),Cells(j,

> > > > 122)).value=Workbooks(FromWorkbook).Sheets(1).Range(Cells(i, 7),
> > > > Cells(i, 127)).value
> > > >
> > > > should do it in the one line for you
> > > >
> > > >
> > > > fullers wrote:
> > > > > I have the following code that works fine and does what I need it
to do. What
> > > > > I am wondering is if there is a way to improve the code inside the
if
> > > > > statement so that the 2 workbooks do not need to be alternately
selected.
> > > > > Thus I hope speed up the macro.
> > > > >
> > > > > The bit inside takes a range of 120 cells in one row from
"FromWorkbook"
> > > > > then selects "ToWorkbook" and the first cell where the range is to
be pasted
> > > > > and then pastes the data.
> > > > >
> > > > > If Workbooks(ToWorkbook).Sheets("LTDIS pay").Cells(j, 1).Value =
> > > > > Workbooks(FromWorkbook).Sheets(1).Cells(i, 3).Value, 2) Then
> > > > >
> > > > > Workbooks(FromWorkbook).Sheets(1).Activate
> > > > > Range(Cells(i, 7), Cells(i, 127)).Copy
> > > > > Workbooks(ToWorkbook).Sheets("LTDIS pay").Activate
> > > > > Cells(j, 2).Select
> > > > > ActiveSheet.Paste
> > > > >
> > > > > End If
> > > > >
> > > > > Many thanks in advance.
> > > >
> > > >
> >
> >