0

I have a csv file (which is an export from a printer), which is not nicely readable and it also has a lot of information which is not needed. So I had written a VBA macro to copy the relevant data to a second sheet.

With the vba code, I'm looking in worksheet 1 for specific text phrases which will be copied to worksheet 2. These text phrases are not in a specific column in worksheet 1, so I have to look through the complete sheet 1. My code is working, but I have one problem which is to tricky for me and that's why I need your help.

In my case, I'm looking for the phrase "Druckbeginn =" (this is German and means Printstart) and is followed by a unix timecode. In most rows, the phrase "Druckbeginn =" exists, but in some it does not.

So lets say in row 1, 2 and 4 the phrase is existing but in row 3 not. The problem is, that my vba code copies the cells from worksheet one to worksheet two, but it copies into the row 1 ,2 and 3 and not to 1, 2 and 4.

If you need my csv file and/or my complete vba code, please let me know.

this is my code:

w1.Activate
For Each r In Intersect(Range("A:AS"), ActiveSheet.UsedRange)
   v = r.Value
   If InStr(v, "Druckbeginn =") > 0 Then
      r.Copy w2.Cells(B, 3)
      B = B + 1
   End If
Next r
nicolaus-hee
  • 787
  • 1
  • 9
  • 25
slekk
  • 29
  • 1
  • 9
  • 2
    Move `B = B + 1` down one line, out of the `if` clause. That way, `B` will be increased even when "Druckbeginn" is not found and the next time it is, it will be copied to the respective line in `w2`. – nicolaus-hee Jun 05 '15 at 09:05
  • This is not working, because it don't finds the correct cells from worksheet one. – slekk Jun 05 '15 at 09:21
  • I uploaded now the csv and the macro to [Dropbox](https://dl.dropboxusercontent.com/u/14554224/CSV_Macro.zip) – slekk Jun 05 '15 at 09:22

1 Answers1

0

Solution: This code worked for me:

' Insert variable declarations etc.
Set w1 = ThisWorkbook.Sheets("Sheet1")
Set w2 = ThisWorkbook.Sheets("Sheet2")
For Each r In Intersect(w1.Range("A:AS"), w1.UsedRange)
    If InStr(r.Value, "Druckbeginn =") > 0 Then
        ' The important change is in the next line
        w2.Cells(r.Row, 3).Value = r.Value
    End If
Next r

Result: (before obtaining the sample data so looking a bit more simple)

Sheet 1:

enter image description here

Sheet 2 (after running the macro):

enter image description here

Explanation of the solution: In your code, your are only increasing B when the string is found resulting in all the copied cells to be copied to the top of the column in your results sheet instead of to the corresponding line in the source data. Instead, we can simply refer to the .Row attribute of the source data. r.Row is the row number of the current r element of your For Each loop and since you want the cell to be in the same row number in ws2, you can use the same row number.

Explanation of the code improvements:

(1) For Each r In Intersect(w1.Range("A:AS"), w1.UsedRange): To avoid confusion about which worksheet the .Range is referring to, I added the w1. specifier.

(2) If InStr(r.Value, "Druckbeginn =") > 0 Then: Instead of assigning v = r.Value you can just use r.Value in your If clause directly since you are not using it again later.

(3) w2.Cells(r.Row, 3).Value = r.Value: Copy and paste actions require jumping between worksheets which is worse in terms of performance than directly assigning a value to a cell through its .Value attribute, especially when you are going through large files.

nicolaus-hee
  • 787
  • 1
  • 9
  • 25
  • please download my file from Dropbox, your solution is not working – slekk Jun 05 '15 at 09:29
  • Ok. I'm looking at it now. – nicolaus-hee Jun 05 '15 at 09:36
  • I edited my answer (`w2.Cells(r.Row, 3).Value = r.Value`). My mistake was that the code is going through rows AND columns, hence `B` would be increased horizontally and vertically. Instead of `B`, we can just use the row number of the source cell. This definitely works for me, please confirm :-) – nicolaus-hee Jun 05 '15 at 10:02
  • yes great it works now, thanks for your very quick help – slekk Jun 05 '15 at 10:41