ExcelBanter

ExcelBanter (https://www.excelbanter.com/)
-   Excel Programming (https://www.excelbanter.com/excel-programming/)
-   -   Improve macro help (https://www.excelbanter.com/excel-programming/364928-improve-macro-help.html)

fullers

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.

[email protected]

Improve macro help
 
(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).Rang e(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

Improve macro help
 
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

" 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).Rang e(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.




[email protected]

Improve macro help
 
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

" 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).Rang e(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

Improve macro help
 
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


" 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

" 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).Rang e(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.





[email protected]

Improve macro help
 

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


" 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

" 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).Rang e(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.






Bob Phillips

Improve macro help
 
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" wrote in message
...
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


" 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

" 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).Rang e(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.








All times are GMT +1. The time now is 08:08 AM.

Powered by vBulletin® Copyright ©2000 - 2025, Jelsoft Enterprises Ltd.
ExcelBanter.com