Как сделать l oop через столбец ячеек и записать в другой столбец ячеек - PullRequest
1 голос
/ 13 марта 2020

В моей книге у меня есть несколько листов данных столбцов, и я записываю данные на целевой лист с двумя столбцами объединенных данных, и это прекрасно работает. Моя проблема в том, что я затем через первый столбец дат провожу oop и пытаюсь записать название дня в столбце 3 (для сводной таблицы). Код зависает после записи первых 50 или около того ячеек (из 1240). For for l oop содержит проблему, которая, кажется, указывает на переполнение некоторой переменной. Вот мой код:

Sub copycolumn()
Dim lastrow, erow As Integer
Dim I As Long
Dim data As String
Dim Assets As Variant
Dim Asset As Variant

With Sheets("Sheet1") 'Clear the existing sheet rows
 lastrow = Worksheets("Sheet1").Cells(Rows.Count, 1).End(xlUp).Offset(1, 0).Row
 .Range(.Cells(2, 1), .Cells(lastrow, 1)).ClearContents
 .Range(.Cells(2, 2), .Cells(lastrow, 1)).ClearContents
 .Range(.Cells(2, 3), .Cells(lastrow, 1)).ClearContents
End With

Assets = Array("Water 2016", "Water 2017", "Water 2018", "Water 2019", "Water 2020")

For Each Asset In Assets   'copy each sheet's 2 col data to "sheet1" into 1 long column
 With Sheets(Asset)
 lastrow = Application.Max(4, .Cells(.Rows.Count, "A").End(xlUp).Row)
 .Range(.Cells(2, 1), .Cells(lastrow, 1)).Copy 'date
 erow = Worksheets("Sheet1").Cells(Rows.Count, 1).End(xlUp).Offset(1, 0).Row
 Worksheets("Sheet1").Range("A" & erow).PasteSpecial xlPasteValues

 .Range(.Cells(2, 4), .Cells(lastrow, 4)).Copy 'data
 erow = Worksheets("Sheet1").Cells(Rows.Count, 2).End(xlUp).Offset(1, 0).Row
 Worksheets("Sheet1").Range("B" & erow).PasteSpecial xlPasteValues
End With
Next Asset

'goto sheet1 and put day name into column 4
Sheets("Sheet1").Activate 
lastrow = Worksheets("Sheet1").Cells(Rows.Count, 2).End(xlUp).Offset(0, 0).Row
For I = 2 To lastrow 'DeS' hangs in this loop
  Cells(I, 3) = Format(Cells(I, 1), "dddd")
Next
Cells(lastrow, 4).Select

MsgBox "Copied" & vbTab & lastrow & vbTab & "Rows"
End Sub

Где я ошибаюсь? Похоже, это должно быть прямо вперед.

1 Ответ

3 голосов
/ 13 марта 2020

3 вещи, которые я сразу вижу, которые могут вызвать проблемы и должны быть исправлены:

  1. Если вы Dim lastrow, erow As Integer только erow - Integer, но lastrow - Variant , В VBA необходимо указать тип для каждой переменной , или по умолчанию это Variant. Кроме того, в Excel имеется больше строк, чем может обработать Integer, поэтому вам нужно использовать Long:

    Dim lastrow As Long, erow As Long. 
    

    Далее я рекомендую всегда использовать Long , поскольку использование * не дает никаких преимуществ Integer в VB.

  2. Прекратить использование .Activate и .Select. Это очень плохая практика и приводит ко многим ошибкам. См. Как избежать использования Select в Excel VBA . Всегда обращайтесь непосредственно к своей книге и листу. Убедитесь, что все Cells, Range, Rows и Columns имеют ссылку на лист. Некоторые из них без Cells(I, 3) следует изменить на что-то вроде Sheets("Sheet1").Cells(I, 3) или при использовании блока With на .Cells(I, 3).

  3. Вы смешиваете Sheets и Worksheets во всем коде. Убедитесь, что вы знаете разницу. Все листы являются листами, но листы могут быть листом или диаграммой или…

    Поэтому убедитесь, что вы используете Worksheets, так как листы будут намного чище.

    Рекомендую также не повторять Worksheets("Sheet1") все время. Если имя вашего листа изменится с Sheet1 на что-то полезное, например MyRawData, его нужно менять везде. Лучше определить переменную Dim wsData As Worksheet и Set wsData = ThisWorkbook.Worksheets("Sheet1"), тогда вы можете использовать ее как wsData.Range("A1")…

Попытайтесь исправить это и проверить, не застряли ли вы в коде. Если это не решает ваши проблемы, отредактируйте ваш код в вопросе на обновленный. Попытайтесь выяснить, какая строка вызывает проблему, и скажите нам, какая это строка.

Чистая версия вашего кода может выглядеть так:

Option Explicit 'make sure you use it in every module as first line to force proper variable declaration

Public Sub CopyColumn()
    Dim wsData As Worksheet 'name your sheet only once and set a reference using a variable
    Set wsData = ThisWorkbook.Worksheets("Sheet1")

    With wsData 'Clear the existing sheet rows
        Dim LastRow As Long
        LastRow = .Cells(.Rows.Count, 1).End(xlUp).Offset(1, 0).Row
        'the other 2 ClearContents are already covered by this one and therefore are not needed
        .Range(.Cells(2, 3), .Cells(LastRow, 1)).ClearContents
    End With

    Dim Assets As Variant
    Assets = Array("Water 2016", "Water 2017", "Water 2018", "Water 2019", "Water 2020")

    Dim Asset As Variant
    For Each Asset In Assets   'copy each sheet's 2 col data to "sheet1" into 1 long column
        With ThisWorkbook.Worksheets(Asset)
            LastRow = Application.Max(4, .Cells(.Rows.Count, "A").End(xlUp).Row)
            .Range(.Cells(2, 1), .Cells(LastRow, 1)).Copy 'date

            Dim eRow As Long
            eRow = wsData.Cells(wsData.Rows.Count, 1).End(xlUp).Offset(1, 0).Row
            wsData.Range("A" & eRow).PasteSpecial xlPasteValues

            .Range(.Cells(2, 4), .Cells(LastRow, 4)).Copy 'data
            eRow = wsData.Cells(wsData.Rows.Count, 2).End(xlUp).Offset(1, 0).Row
            wsData.Range("B" & eRow).PasteSpecial xlPasteValues
        End With
    Next Asset

    'goto sheet1 and put day name into column 4
    LastRow = wsData.Cells(wsData.Rows.Count, 2).End(xlUp).Row

    Dim i As Long
    For i = 2 To LastRow 'DeS' hangs in this loop
        wsData.Cells(i, 3).Value = Format$(wsData.Cells(i, 1), "dddd")
    Next i

    'jump to the last row
    wsData.Activate
    wsData.Cells(LastRow, 4).Select 'not needed if you don't want explicitly the user to see this

    MsgBox "Copied" & vbTab & LastRow & vbTab & "Rows", vbInformation, "Copy Rows"
End Sub

Обратите внимание, что я не сделал копаться в процессе того, что делает код. Я только что проверил стиль кодирования и исправил синтаксис, где вещи, очевидно, могли бы go ошибаться.

Чем ближе вы придерживаетесь хорошего форматирования и хорошего стиля кодирования, тем меньше ошибок вы получите. Даже если это выглядит иногда немного больше работы, в конце вы сэкономите много времени, не пытаясь найти причину странных проблем.


Дальнейшие мысли

Эта строка

Assets = Array("Water 2016", "Water 2017", "Water 2018", "Water 2019", "Water 2020")

Похоже, вам нужно будет снова покопаться в коде 2021 и добавить "Water 2021", потому что ваш код перестал работать.

Избегайте написания кода, который нужно корректировать каждый год. Я бы рекомендовал l oop - всем рабочим листам и проверить, совпадает ли их имя с "Water ####", чтобы выполнить код для них:

Dim Asset As Worksheet
For Each Asset In ThisWorkbook.Worksheets
    If Asset.Name Like "Water ####" Then
        'your code here …
    End If
End If

Это будет применяться к каждому лист, который называется "Water ####"

...