1
\$\begingroup\$

The below code is working uses formula to concatenate the result from Range("K:M") and write the result in column H.

Is there a better way to write the below code that can concatenate the result to the exact location as per below table? Please note that data in the before table is discontinued but sometimes will be from row 4 to 1000 rows down or the below example.

Sub GenerateStyleFabricColourV4()
 Dim srcData As Range
 Dim rowNum As Long, lastRow As Long
 With Worksheets(2)
 lastRow = Application.Max(4, _
 .Cells(.Rows.Count, "K").End(xlUp).Row, _
 .Cells(.Rows.Count, "L").End(xlUp).Row, _
 .Cells(.Rows.Count, "M").End(xlUp).Row)
 With .Cells(4, "H").Resize(lastRow - 4 + 1, 1)
 .FormulaR1C1 = "=rc[3]&rc[4]&rc[5]"
 .Value = .Value2
 End With
 End With
End Sub
Sub ClearAllData()
Rows("4:" & Rows.Count).ClearContents
End Sub
asked Apr 10, 2017 at 5:20
\$\endgroup\$
5
  • \$\begingroup\$ I suppose it works for you, but it's very difficult for us to review it without knowing your data. (e.g. depending on your data table, you may use range.currentregion instead of the multiple end(xlup) calls. \$\endgroup\$ Commented Apr 10, 2017 at 11:20
  • \$\begingroup\$ @MátéJuhász Will post the data as soon as i get home sorry about that. \$\endgroup\$ Commented Apr 10, 2017 at 11:32
  • 1
    \$\begingroup\$ Would you clearly say what's your question? \$\endgroup\$ Commented Apr 10, 2017 at 14:44
  • \$\begingroup\$ @MátéJuhász Just updated the data table. \$\endgroup\$ Commented Apr 10, 2017 at 18:42
  • \$\begingroup\$ @QuickSilver If my answer resolve the problem for you please consider reviewing it and marking as accepted to help the other users find their answers later on. \$\endgroup\$ Commented Apr 10, 2017 at 20:44

2 Answers 2

3
\$\begingroup\$

To answer another question I provided some helpful information about optimization in vba here, VBA syntax that compiles sub data into master sheet.

This maybe help you to skip the blank rows (You did not clarify your question but based on the data you provided I think that would be your problem) and find the actual last row used. It is in reference to this post. Here's the code:

Dim srcData As Range, rng As Range
Dim rowNum As Long, lastRow As Long
Dim wsh As Worksheet
Sub GSFC_V4()
 Set wsh = Worksheets(2)
 
 wsh.Activate
 Set srcData = Range("K:M")
 Set rng = Range("K4")
 With wsh 
 If Application.WorksheetFunction.CountA(.Cells) <> 0 Then
 lastrow = srcData.Find(What:="*", _
 After:=rng, _
 Lookat:=xlPart, _
 LookIn:=xlFormulas, _
 SearchOrder:=xlByRows, _
 SearchDirection:=xlPrevious, _
 MatchCase:=False).Row
 Else
 lastrow = 1 
 End If
 With .Cells(4, "H").Resize(lastRow - 4 + 1, 1)
 .FormulaR1C1 = "=rc[3]&rc[4]&rc[5]"
 .Value = .Value2
 End With
 End With
End Sub

This is what I am using to optimize my macros. I call OptimizeVBA True at the beginning of each macro and at the end call OptimizeVBA False to set everything back to default. It increases the performance remarkably.

Sub OptimizeVBA(isOn As Boolean)
 Application.Calculation = IIf(isOn, xlCalculationManual, xlCalculationAutomatic)
 Application.EnableEvents = Not (isOn)
 Application.ScreenUpdating = Not (isOn)
 Application.StatusBar = Not (isOn)
End Sub

You need to be careful of how you refer to ranges to avoid getting errors.

answered Apr 10, 2017 at 20:37
\$\endgroup\$
2
  • \$\begingroup\$ Gives a run time error 1000 on the ` With .Cells(4, "H").Resize(lastRow - 4 + 1, 1)` can't work out why. \$\endgroup\$ Commented Apr 14, 2017 at 7:34
  • \$\begingroup\$ @QuickSilver This is not the optimized way to do it but you can try this: before that "With block" add this .Cells(4, "H").Select and substitute the .Cells(4,"H"). with Selection.. You may need to put the second with block out of the first one (i.e. With wsh). \$\endgroup\$ Commented Apr 14, 2017 at 13:19
1
\$\begingroup\$

I had an old script on hand that accomplished most of what I think you're after, though it uses a different approach. I modified it and it seems to work with your data. You'll need to update at least a couple values in the code — see comments.

Option Explicit
Sub GSFC_V4()
Application.ScreenUpdating = False
Dim wb As Workbook
Set wb = Workbooks("ConcatTest.xlsx") 'update workbook name
Dim ws As Worksheet
Set ws = wb.Sheets("Sheet1") 'update worksheet name
Dim TextS As String
Dim TextF As String
Dim TextC As String
Dim StartRow As Long
StartRow = 5 'assumes row 5 is the first row containing data to be concatenated
Dim LastRow As Long, lRow1 As Long, lRow2 As Long, lRow3 As Long
lRow1 = Range("K" & Rows.Count).End(xlUp).Row
lRow2 = Range("L" & Rows.Count).End(xlUp).Row
lRow3 = Range("M" & Rows.Count).End(xlUp).Row 
LastRow = WorksheetFunction.Max(lRow1, lRow2, lRow3)
Dim CheckCell As Range
Dim ConcatRange As Range
Set ConcatRange = Range(ws.Cells(StartRow, "K"), ws.Cells(LastRow, "K"))
Dim i As Long
i = StartRow
For Each CheckCell In ConcatRange
 TextS = ws.Cells(i, "K").Value
 TextF = ws.Cells(i, "L").Value
 TextC = ws.Cells(i, "M").Value
 i = i + 1
 If Not CheckCell.Value = "" Then
 CheckCell.Offset(0, -3).Value = TextS & TextF & TextC
 End If
Next CheckCell
Application.ScreenUpdating = True
End Sub
M--
2554 silver badges12 bronze badges
answered Apr 10, 2017 at 22:48
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.