My code works, but Excel stops responding. No erros mgs from the code

Question

I need some help to make my conde much simpler. I'm starting to code on VBA and build my own scripts and they work properly, sometimes. But they are always tooooo big and much more complicated than it could be.

This is one case that everytime I run the script, Excel crashes. Can someone assist me in making this code much more simple?

    Sub Cleaning_Mirexs()

    Application.ScreenUpdating = False

    Dim UltCel As Range
    Dim Mirex As String
    Dim Glip As String

Mirex = "S"
Glip = "UP"

Set UltCel = Cells(Rows.Count, 2).End(xlUp)

' Moving Data for treatment

    Range("R2").Select
    Range(Selection, Selection.End(xlDown)).Select
    Selection.Copy
    Range("X2").Select
    Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
        :=False, Transpose:=False
    Application.CutCopyMode = False
    Selection.TextToColumns Destination:=Range("X2"), DataType:=xlDelimited, _
        TextQualifier:=xlDoubleQuote, ConsecutiveDelimiter:=False, Tab:=False, _
        Semicolon:=False, Comma:=False, Space:=False, Other:=True, OtherChar _
        :="-", FieldInfo:=Array(Array(1, 1), Array(2, 1)), TrailingMinusNumbers:=True

' Mirex Formicide Data

Range("$Y2").Select

    Do While ActiveCell <> UltCel
    If InStr(1, ActiveCell.Text, Mirex) Then
    ActiveCell.FormulaR1C1 = ""
    ActiveCell.Offset(0, -1).Select
    ActiveCell.Clear
    ActiveCell.FormulaR1C1 = "IS FORMICIDA MIREX-S" & ActiveCell.Value
    ActiveCell.Offset(1, 1).Select

    ElseIf ActiveCell.Offset(xlDown) Then

    End If

    Loop

' Glip Herbicide Data

Range("Y2").Select

    Do While ActiveCell <> UltCel
    If InStr(1, ActiveCell.Text, Glip) Then
    ActiveCell.Formula = ""
    ActiveCell.Offset(0, -1).Select
    ActiveCell.Clear
    ActiveCell.FormulaR1C1 = "HB GLIP-UP" & ActiveCell.Value
    ActiveCell.Offset(1, 1).Select

    ElseIf ActiveCell.Offset(1, 0).Select Then

    End If

    Loop

' Light Tractor Data
Range("X2").Select

    Do While ActiveCell <> UltCel
    If InStr(1, ActiveCell.Text, "Tratores leves") Then
    ActiveCell.Clear
    ActiveCell.FormulaR1C1 = "Tratores leves" & ActiveCell.Value
    ActiveCell.Offset(1, 0).Select

    ElseIf ActiveCell.Offset(1, 0).Select Then

    End If

    Loop

' Heavy Tractor Data
Range("X2").Select

    Do While ActiveCell <> UltCel
    If InStr(1, ActiveCell.Text, "Tratores pesados") Then
    ActiveCell.Clear
    ActiveCell.FormulaR1C1 = "Tratores pesados" & ActiveCell.Value
    ActiveCell.Offset(1, 0).Select

    ElseIf ActiveCell.Offset(1, 0).Select Then

    End If

    Loop

' back to original place after data treatment
Range("X2").Select
    Range(Selection, Selection.End(xlDown)).Select
    Selection.Copy
    Range("X2").Select
    Selection.PasteSpecial Paste:=xlPasteValues


Application.ScreenUpdating = True

MsgBox "Success!"

End Sub

I would like the code to run everything at once, but they way I wrote the script, feels like, a individual run for each data set.

Well, here it is! Let's have fun :)

Thank you!

Maria

Hi Maria, thank you for your question. Could you please elaborate on "Excel crashes" because that is really too vague of a problem description for us. At what point in the code do things go wrong, is there an error message? — Luuklag, May 03 '18 at 13:52
Hmm, Range is R2 but ultcell is in column C if I am reading this. Probably crashing due to not being able to terminate that while loop? — Jacob H, May 03 '18 at 13:53
You want to avoid using select. This will help: https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba?rq=1 — Harassed Dad, May 03 '18 at 13:54
A wall of code together with "sometimes it crashes" isn't a good question. Please give a [mcve]. We shouldn't need to discover by trial and error the condition under which it "crashes" (whatever that means). — John Coleman, May 03 '18 at 13:55
@Luuklag So, no erros msg. Just goes blank with the hourglass, like the "not responding" situation. :( — Maria Richter, May 03 '18 at 13:57
@MariaRichter That's often a result of a never ending `Do Loop` - you're best off moving away from that and giving your loops more explicit criteria. — dwirony, May 03 '18 at 13:58
@JohnColeman Hi! I´m sorry if it wasn't so clear. I´ll do better in my next question! Thanks for the feedback, always helpfull. — Maria Richter, May 03 '18 at 14:01
In addition to cleaning up the selects and activecells, step through your code line-by-line and be sure it does **exactly** what you want it to. Watch what happens on the sheet after each line of code. That will surely show you were the error occurs (in this case, most likely the infinite loop) and then you can work on fixing that. — Scott Holtzman, May 03 '18 at 14:02
@ScottHoltzman Yeah, I did that. Everything is running fine! But thank you for stopping by and giving me this tip! :) — Maria Richter, May 03 '18 at 14:04
`Do While ActiveCell <> UltCel` Are you sure this can happen? As @dwirony said, your loop is probably never ending. — Foxfire And Burns And Burns, May 03 '18 at 14:05
@MariaRichter You can always hit 'Esc' and debug your macro to see what is happening in that moment. Or use conditional breakpoint. — GSazheniuk, May 03 '18 at 14:06
Ok, let's try something. Your line `Do While ActiveCell <> UltCel`, this is a comparison between the values in those cells. If those values are text, **in this case VBA is case sensitive**, and that cause a never ending loop. Try better with `Do While Ucase(ActiveCell) <> Ucase(UltCel)`. If those cells are numbers, forget what I said right now xD. Just trying to help. — Foxfire And Burns And Burns, May 03 '18 at 14:13
@FoxfireAndBurnsAndBurns Thank you! They are not numbers and will try that! ;) Let you know in a sec if that worked — Maria Richter, May 03 '18 at 14:16

dwirony · Answer 1 · 2018-05-03T19:24:51.893

Okay, I took a stab at trying to fix this up, but I have several questions about what you're trying to accomplish here... For example:

ActiveCell.Clear
ActiveCell.FormulaR1C1 = "Tratores pesados" & ActiveCell.Value

Here you're just clearing your ActiveCell, then adding some text followed by the ActiveCell.Value which is now nothing, since you just cleared it. I'm not sure what your intent is there.

You also have

ElseIf ActiveCell.Offset(1, 0).Select Then
End If

Which I don't think has ANY functionality, and I'm confused just trying to understand why this would be necessary so I omitted it.

I've also gotten rid of your Do/Loops and replaced them with For loops, which are much more reliable. I've also gotten rid of Select/Activate for the most part, as those are inefficient.

I've also changed UltCel to a Long for the For loops.

If anyone else wants to edit this go right ahead, I'm sure there's something I've missed (like I'm not sure about the .TextToColumns bit:

Sub Cleaning_Mirexs()

Application.ScreenUpdating = False

Dim UltCel As Long
Dim Mirex As String, Glip As String
Dim i As Long

Mirex = "S"
Glip = "UP"

UltCel = Cells(Rows.Count, 2).End(xlUp)

'Moving Data for treatment
Range("X2:X" & UltCel).Value = Range("R2:R" & UltCel).Value
Range("X2:X" & UltCel).TextToColumns Destination:=Range("X2"), DataType:=xlDelimited, _
TextQualifier:=xlDoubleQuote, ConsecutiveDelimiter:=False, Tab:=False, _
Semicolon:=False, Comma:=False, Space:=False, Other:=True, OtherChar _
:="-", FieldInfo:=Array(Array(1, 1), Array(2, 1)), TrailingMinusNumbers:=True

For i = 2 To UltCel
    If InStr(Range("X" & i).Value, Mirex) Then
        Range("X" & i).Value1 = "IS FORMICIDA MIREX-S"
    ElseIf InStr(Range("X" & i).Value, Glip) Then
        Range("X" & i).Value = "HB GLIP-UP"
    ElseIf InStr(Range("X" & i).Value, "Tratores leves") Then
        Range("X" & i).Value = "Tratores leves"
    ElseIf InStr(Range("X" & i).Value, "Tratores pesados") Then
        Range("X" & i).Value = "Tratores pesados"
    End If
Next i

For i = 2 To UltCel
    If InStr(Range("Y" & i).Value, Mirex) Then
        Range("Y" & i).Value1 = "IS FORMICIDA MIREX-S"
    ElseIf InStr(Range("Y" & i).Value, Glip) Then
        Range("Y" & i).Value = "HB GLIP-UP"
    ElseIf InStr(Range("Y" & i).Value, "Tratores leves") Then
        Range("Y" & i).Value = "Tratores leves"
    ElseIf InStr(Range("Y" & i).Value, "Tratores pesados") Then
        Range("Y" & i).Value = "Tratores pesados"
    End If
Next i

'back to original place after data treatment
Range("X2:X" & UltCel).Value = Range("X2:X" & UltCel).Value
Range("Y2:Y" & UltCel).Value = Range("Y2:Y" & UltCel).Value

Application.ScreenUpdating = True

MsgBox "Success!"

End Sub

Hey @dwirony thank you so much for your help! Let me see if I can answer your questions! My cells are written like this **[CTP016] Tratores leves** and I need them to be like this **Tratores Leves**, this is why I clear and replace with a new value. As for the ElseIf, it´s beacuse my data is distributed in the Y and X colum, from line 2 to line 3973, so I need to evaluate every data in this range. Does it makes sense? — Maria Richter, May 03 '18 at 14:32
@MariaRichter I'm sorry I still don't understand - so you don't need `Clear` because if you're putting a new value in the cell, it will just overwrite the old value - no need to clear it. And your `ElseIf` is evaluating a `Select` statement, which actually looks like it will always return True - but even if it DOES return True, nothing happens.... — dwirony, May 03 '18 at 14:46
Yeah, about the clear thing, you´re correct, I can just overwrite. The thing I want with the `if` is that: If the cell does not have the value I want, then I want to move to cell bellow, so I can start the analysis again. If the cell have the value I want, I overwrite the value. — Maria Richter, May 03 '18 at 14:47
@MariaRichter Could you maybe include a screenshot of your data, before and after? — dwirony, May 03 '18 at 14:48
[The data](https://docs.google.com/spreadsheets/d/1gzCs7a1lyt7DwkrsDzGu2X4Z5QGFom9fs_l7yC6b07U/edit?usp=sharing) — Maria Richter, May 03 '18 at 14:55
@Maria It's behind permissions and I'd rather not go through that requseting process. Can you just take a screenshot and embed an image? — dwirony, May 03 '18 at 15:05
done! The colum select is the data that I have. The XY and Z column is the data after the `Range("X2:X" & UltCel).Value = Range("R2:R" & UltCel).Value Range("X2:X" & UltCel).TextToColumns Destination:=Range("X2"), DataType:=xlDelimited, _ TextQualifier:=xlDoubleQuote, ConsecutiveDelimiter:=False, Tab:=False, _ Semicolon:=False, Comma:=False, Space:=False, Other:=True, OtherChar _ :="-", FieldInfo:=Array(Array(1, 1), Array(2, 1)), TrailingMinusNumbers:=True` — Maria Richter, May 03 '18 at 16:16
@MariaRichter Okay I think I understand... But what's the point of `Mirex` and `Glip`? Also, why search column Y? It looks like it never contains anything worth changing. I've altered the code again above. — dwirony, May 03 '18 at 19:24

My code works, but Excel stops responding. No erros mgs from the code

1 Answers1