0

I have a macro named Splittext that is called when there is a change in cell "B4" of sheet Macro Process it doesn't work when I call it but it works when I manually run it. There is no error in the code

Sub splitText()
    Dim wsS1 As Worksheet 'Sheet1
    Dim textstring As String, warray() As String, counter As Integer, strg As String

    Set wsS1 = Sheets("OUTPUT 1")
    wsS1.Activate

    textstring = Range("A2").Value
    warray() = Split(textstring, ">")

    For counter = LBound(warray) To UBound(warray)
        strg = warray(counter)
        Cells(counter + 3, 1).Value = Trim(strg)
    Next counter

    textstring = Range("B2").Value
    warray() = Split(textstring, ">")

    For counter = LBound(warray) To UBound(warray)
        strg = warray(counter)
        Cells(counter + 3, 2).Value = Trim(strg)  
    Next counter

    textstring = Range("C2").Value
    warray() = Split(textstring, ">")

    For counter = LBound(warray) To UBound(warray)
        strg = warray(counter)
        Cells(counter + 3, 3).Value = Trim(strg)
    Next counter
End Sub

This code is supposed to separate the text present in the Cells ("A2")("B2")("C2") of sheet "OUTPUT 1"

This is how I am calling the code

Private Sub Workbook_SheetChange(ByVal Sh As Object, ByVal Target As Range)
    Set Target = Range("B4")
    If Target.Value = "Completed" Then
        Call splitText
    End If
End Sub
Pᴇʜ
  • 56,719
  • 10
  • 49
  • 73
j.doe
  • 43
  • 9
  • Exactly how are you calling it automatically? – Tim Williams Jan 23 '19 at 05:55
  • @TimWilliams please check the question i've updated it with the code that i am using to call the macro – j.doe Jan 23 '19 at 05:58
  • 2
    None of your code refers to specific worksheets - a good start would be to qualify all of your range/cells references so they refer to the correct worksheet. Which sheet are you monitoring for changes? All of them? – Tim Williams Jan 23 '19 at 06:01
  • FWIW You have code that is essentially repeating at the top which, I think, with another outer loop from 1 to 3 could reduce the inner looping to just one block. For i = 1 to 3: textstring = Cells(2,i).Value : Cells(counter + 3, i) '<== key replacement lines – QHarr Jan 23 '19 at 06:02
  • you mean i need to define a worksheet on which the code needs to run??? @TimWilliams – j.doe Jan 23 '19 at 06:02
  • Yes - otherwise it just defaults to whichever sheet is active. https://stackoverflow.com/questions/28439376/what-is-the-default-scope-of-worksheets-and-cells-and-range/28439984#28439984 – Tim Williams Jan 23 '19 at 06:03
  • Ill try and get back to you – j.doe Jan 23 '19 at 06:04
  • @TimWilliams i've tried that as well still it didnt work even after defining the worksheet – j.doe Jan 23 '19 at 06:10
  • In that case it would be useful to edit your question to update your code with the version which specifies the worksheet. – Tim Williams Jan 23 '19 at 06:13
  • updated the code @TimWilliams please check – j.doe Jan 23 '19 at 06:14
  • Huge problems in code: (1) Calling `SheetChange` and then setting the `Target` without checking what cell actually changed. (2) activating sheets. (3) not properly qualifying ranges. – AJD Jan 23 '19 at 06:40

1 Answers1

1

It's unclear what sheet you're monitoring for changes, but this worked for me:

Private Sub Workbook_SheetChange(ByVal Sh As Object, ByVal Target As Range)
    Set Target = sh.Range("B4")
    If Target.Value = "Completed" Then
        Application.EnableEvents = False
        splitText
        Application.EnableEvents = True
    End If
End Sub


Sub splitText()
    Dim warray() As String, i As Long, c As Range
    For Each c In ThisWorkbook.Sheets("OUTPUT 1").Range("A2:C2").Cells
        warray = Split(c.Value, ">")
        For i = LBound(warray) To UBound(warray)
            c.Offset(i + 1, 0).Value = Trim(warray(i))
        Next i
    Next c
End Sub
Tim Williams
  • 154,628
  • 8
  • 97
  • 125
  • finally ...Thanks! ...it worked ..Thank you so much for your patience and help! – j.doe Jan 23 '19 at 06:30
  • Tim, I know it was in the OP - but why have a `SheetChange` and then set *`Target`* to a fixed address that is likely to be anything but the target? I am really surprised you did not pick this up! (Instead: `If Target.Address = "B4"` etc). – AJD Jan 23 '19 at 06:35
  • I did pick it up but there's some potential reasons for why it might be that way so... – Tim Williams Jan 23 '19 at 06:40
  • Tim, from OP "that is called when there is a change in cell "B4" of sheet " - so the event handler should check to see if that is the cell that is changed - not force the issue. The only thing I can think of is if B4 is changed through formula but then some other criteria must be checked to see if there really is a change. The code as currently stands will run ***every time there is a change in any worksheet*** as long as B4 = completed. – AJD Jan 23 '19 at 06:44
  • The comment above the code was a hint for the OP that they had a bit of work to do to make it right. B4 is right under the cells being split, and the process would quite likely overwrite whatever is in B4... – Tim Williams Jan 23 '19 at 06:47