Excel Crash When Copying row through VBA

Question

Newbie here on Excel VBA. I would just like to ask how to fix the error I'm getting? I'm trying to copy an entire row base on a criteria, and paste it on another sheet (same workbook). Here's my code:

Dim ws As Worksheet
Dim ws1 As Worksheet
Dim endRow As Range

Set ws = Sheets("FIELD OFFICE DATABASE")
Set ws1 = Sheets("Transferred Items")

set endRow = Range("B" & Rows.Count).End(xlUp).Offset(1, 0)

ws.Activate
ws.Unprotect "321321"
Range("B2").Select

Do Until endRow
    If ActiveCell.Value = Me.cmbemn.Text Then
        ActiveCell.Rows("B:S").Select
        Selection.copy
        ws1.Activate
        ws1.Unprotect "321321"
        endRow.Select
        ActiveSheet.Paste
    Else
        ActiveCell.Offset(1, 0).Select
    End If
Loop

What happens is its either my Excel crashes, or an error pops up saying "Object required". But mostly it crashes.

Please help! Thanks!

endRow is using the activesheet implicitly. Which sheet should you be finding the endRow in? ws? — QHarr, Jan 23 '18 at 09:06
You should avoid using `Select`or `ActiveCell`. See [this post](http://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba-macros) — Vincent G, Jan 23 '18 at 09:07
You should change Range("B" & Rows.Count).End(xlUp).Offset(1, 0) to a more concrete end value for your loop. — QHarr, Jan 23 '18 at 09:08

QHarr · Answer 1 · 2018-01-23T09:31:58.080

0

Try changing the loop termination condition to something like:

Do Until ActiveCell.Address = endRow.Address

I think you could terminate earlier with:

Set endRow = ws.Range("B" & Rows.Count).End(xlUp)

You also should be explicit in which worksheet you are working with or else the currently activesheet will be used. I have assumed you are working with ws.

And Rows should not specify columns within it:

ActiveCell.Rows("B:S").Select

It should have a Rows specification. Potentially you could do something like:

ActiveCell.EntireRow.Copy

Or as per later comment,

currCell.EntireRow.Copy

There doesn't need to be a .Select then a second line for the copy. You can do in one line.

Note also @Jeeped's comment on Unprotect.

My preference would be to specify a loopRange e.g.

Set loopRange = ws.Range("B2:B" & ws.Range("B" & Rows.Count).End(xlUp).Row)

and then loop over that range.

Dim currCell As Range

For Each currCell in loopRange

    If currCell.Value .....do stuff

Next currCell

This has the added benefit of getting rid of the use of activecell.

edited Jan 23 '18 at 09:31

answered Jan 23 '18 at 09:15

QHarr

83,427
12
54
101

Hi, for the "currCell" variable, what value should I set it with? I am having an error saying object needed or undefined. Thank you! – Kelvs Jan 24 '18 at 02:04
code: Dim ws As Worksheet Dim ws1 As Worksheet Dim wsendRow As Range Dim wsendRow1 As Range Set ws = Sheets("1") Set ws1 = Sheets("2") Set wsendRow = ws.Range("B" & Rows.Count).End(xlUp) Set wsendRow1 = ws1.Range("A" & Rows.Count).End(xlUp) ws.Range("B2").Activate Do Until ActiveCell.Address = wsendRow.Address If ActiveCell.Value = Me.cmbemn.Text Then ActiveCell.Rows("B:S").Copy ws1.Activate wsendRow1.PasteSpecial Else ActiveCell.Offset(1, 0).Select End If Loop – Kelvs Jan 24 '18 at 02:27
Im really sorry. But Im lost at how should I code it. Do you mean this? Set curCell = range("A1")? – Kelvs Jan 26 '18 at 07:42
Hi. Thank you for helping me. I just want to further explain what I'm trying to achieve. There's 2 tables, 1 on diff. sheets. I have a "Transfer" button, this [form](https://imgur.com/UVNEnxH) will appear upon clicking. [These](https://imgur.com/Olyk5uC)are the data I would like to be transfered. As you can see, the columns are different, so what I did is [THIS](https://imgur.com/IbSd9Ul). I know there's a simplier way of doing this, I just don't know how. Please help! Thank you! – Kelvs Jan 29 '18 at 01:32
With that code, this is the error I'm getting: Compile error: Wrong number of arguments or invalid property assignment. and it points on this line of code: .Offset(4, 0).Value = ActiveCell.Column("F").Value Thank you again! – Kelvs Jan 29 '18 at 01:35
1

ActiveCell.Column("F") is incorrect syntax. ActiveCell is a range that has an address e.g. F2. The code above would not produce this error. You might try saying something like: ActiveCell.Offset(4, 0) = ActiveCell. The offset here is setting the current cell value to be the same as the cell 4 rows ahead in the same column. If you meant 4 columns to the right that would be (0,4). – QHarr Jan 29 '18 at 06:10
Hi Sir. Thank you for not giving up no me here. I really appreciate it. I did what you ask, and made several adjustments as well. However, I'm stuck with this [ERROR](https://imgur.com/XOcdXEb). This is my current [CODE](https://imgur.com/E69hk83) sir . I've done several changes already, like unprotecting the sheet, activating it, etc. But still, this error does not go away :(. Please help! Again, thank you for your time and effort. – Kelvs Jan 30 '18 at 03:37
Sorry, I'm converting the codes to images because it does not fit here in comments. Anyways, the error is pointing on the first line after the if statement inside Do. Pointing in "lr.Range(1, 1).Value = Me.cmbemn.Text". Thank you again good Sir! – Kelvs Jan 30 '18 at 06:08
Here are the [CODES](https://pastebin.com/2XXC7dJ7) for the transfer button. Thank you! – Kelvs Jan 30 '18 at 06:12
Yes. The button is within a form. – Kelvs Jan 30 '18 at 06:16
Yes, I do have that object. Also, I've update my code and activate the sheet first before selecting the range. [Here](https://pastebin.com/Fmdx7hF9) is my new code. However, I'm still receiving the error and still pointing in the same code. Also, just to clarify, I'm trying to transfer the data from the field office to the transferred sheet. Afterwards the data from the field office sheet will be deleted. Thank you! – Kelvs Jan 30 '18 at 06:38
If you comment lr.Range(1, 1).Value = Me.cmbemn.Text out, does the error come back at the next line? – QHarr Jan 30 '18 at 07:16
I just tested, and yes, the error comes back at the next line. When I point the mouse cursor in the lr.range, there's no value showing. but for the other one (ie. me.cmbemn.text) there's a value. – Kelvs Jan 30 '18 at 07:18
There won't be a value until after the assignment i.e. if the code worked properly there would be a value showing when you got to the next line. Has this code worked at any point i.e. successfully transferred data? – QHarr Jan 30 '18 at 07:19
No sir it never worked. I also did try using the F8 button to play the code one by one. – Kelvs Jan 30 '18 at 07:27
Can you try the following code making sure there you expect it to run i.e. have a matching value in the box as in the sheet. https://pastebin.com/3VK0q0Jd The .Cells(1, 1) is irrelevant however, it seems. – QHarr Jan 30 '18 at 07:37
I've copy pasted your code and made sure that everything has value. Error still appear on the same line. Maybe there is an alternative way? If you would like, I can explain the process that I am trying to achieve again. Also, thank you very much good sir for not giving up on this. – Kelvs Jan 30 '18 at 08:16
Absolutely sir. Please go ahead. – Kelvs Jan 30 '18 at 08:25
I am going to suggest that you post as a new question or, if you feel this is the same question, unmark my answer as accepted (you can potentially instead mark as helpful with an upvote). Either way be sure to include in the question the things we have tried (in brief); add the screenshot of your data layout; make clear the error description and the line it happens on. I have to nip off now so doing this may get you a faster solution. I will check back in later. – QHarr Jan 30 '18 at 08:30

score 0 · Answer 2 · answered Jan 23 '18 at 09:17

Unprotecting your worksheet cancels the copy/paste operation.

Protect your worksheets with the following parameter,

UserInterfaceOnly:=true
'example:
ws1.Protect password:="321321", UserInterfaceOnly:=true

Now you can do anything you want to do to them in VBA without unprotecting them

Excel Crash When Copying row through VBA

2 Answers2

Linked