[Excel] Runtime Error 1004 in very simple script

Hi I'm getting a Runtime Error 1004 @ the bolded line in the following script and I cannot understand why:

Code:

---

Public Sub MACRO()
    Dim x As Integer
    Dim y As Integer
    Dim n As Integer
    Sheets("Cognos").Select
    For x = 46 To 53
        For y = 13 To 639
            CellValue = Cells(y, x).Text
            Cells(y, x).Select
            If CellValue = "ADD SKU" Then
                Cells(y, 2).Resize(1, 1).Select
                n = ActiveCell.Value
                Sheets("Detail").Select
                NextRow = Cells(Rows.Count, 1).End(xlUp).Row + 1
                Cells(NextRow, 5).Select
                ActiveCell.Value = n             
                Sheets("Cognos").Select
            End If
        Next y
    Next x
End Sub

---

Any help would be greatly appreciated.

Quote:

---

n = ActiveCell.Value
Sheets("Detail").Select
NextRow = Cells(Rows.Count, 1).End(xlUp).Row + 1
Cells(NextRow, 5).Select
ActiveCell.Value = n

---

try

vb Code:

sheets("detail").cells(sheets("detail").rows.count, 1).offset(1, 4).value = activecell.value

better to avoid using active anything, work with fully qualified ranges

Try not to use variables without declaring them first (CellValue, NextRow). Turn on Option Explicit to prevent from doing that.

Code:

---

Cells(y, 2).Resize(1, 1).Select
n = ActiveCell.Value

---

resize(1, 1) does nothing on a single cell. instead of those 2 statements:

Code:

---

n =  Cells(y, 2).Value

---

Hello EmilN :)

Welcome to the forums :wave:

kaliman79912 rightly suggested that you should declare your variables and use the option explicit. I would further suggest you to read this

Topic: To 'Err' is Human - Good Coding Practices
Link: http://siddharthrout.wordpress.com/2...-err-is-human/

There are few problems with the code.

1) Avoid using the Integer type. You have declared n As Integer. What if the 'ActiveCell' Value is 1111111?

To understand the data type, I would recommend this link.

Topic: The Integer, Long, and Byte Data Types
Link: http://msdn.microsoft.com/en-us/library/aa164754%28v=office.10%29.aspx

2) Like Pete correctly suggested, avoid the use of .Select They slow down your code and are also responsible for many errors :)

Consider this example

Quote:

---

Sheets("Detail").Select
NextRow = Cells(Rows.Count, 1).End(xlUp).Row + 1
Cells(NextRow, 5).Select
ActiveCell.Value = n

---

This can be also written as

Code:

---

Option Explicit

Sub Sample()
    Dim ws As Worksheet, NextRow As Long, n As Long
    '
    '~~> Rest of Code
    '
    Set ws = Sheets("Detail")
   
    With ws
        NextRow = .Range("A" & Rows.Count).End(xlUp).Row + 1
        .Range("E" & NextRow).Value = n
    End With
    '
    '~~> Rest of Code
    '
End Sub

---

3) There is no error handling. If you are developing for a client then your client will be left high and dry as the code will break leaving your client clueless as to what exactly went wrong.

The code that you posted in post1 can also be written as

Code:

---

Option Explicit

Public Sub MACRO()
    Dim ws1 As Worksheet, ws2 As Worksheet
    Dim x As Long, y As Long, n As Long
    Dim CellValue As String
   
    On Error GoTo whoa
   
    Application.ScreenUpdating = False
   
    Set ws1 = Sheets("Cognos")
    Set ws2 = Sheets("Detail")
   
    With ws1
        For x = 46 To 53
            For y = 13 To 639
                CellValue = .Cells(y, x).Text
                If CellValue = "ADD SKU" Then
                    n = .Cells(y, 2).Value
                    NextRow = ws2.Range("A" & Rows.Count).End(xlUp).Row + 1
                    ws2.Cells(NextRow, 5).Value = n
                End If
            Next y
        Next x
    End With
   
LetsContinue:
    Application.ScreenUpdating = True
    Exit Sub
Err:
    MsgBox Err.Description
    Resume LetsContinue
End Sub

---

Hope this helps :)

Sid