2

I am trying to copy some cells in the same sheet if there's a "-" or a "/" in a certain cell.

According to the number of "-" or "/" is the number of times it is going to copy. This is my code but it's not working, can anyone help?

Sub TWB_Copy_columns()
'TWB_Copy_columns Macro

Dim celltxt As String
Range("B14").Select
Selection.End(xlToRight).Select
celltxt = Selection.Text
If InStr(1, celltxt, "-") Or InStr(1, celltxt, "/") Then
    Range("BA5:BB36").Select
    Selection.Copy
    Range("BD5").Select
    ActiveSheet.Paste
    Range("BG5").Select
End If

End Sub
  • What does it do instead of work? – Tim Williams Apr 26 '17 at 14:59
  • 1
    Could you please be a bit more specific? What do you mean when you say that its not working? Is the code running with an error? If so, what's the error code / message and on which line is the code breaking? Or is the code running (without an error) but merely not doing what you expected it to do? If that's the case then please share with us what it is doing (instead) and what the expected outcome is / would be. Maybe some screenshots and sample data can help in this respect. – Ralph Apr 26 '17 at 15:00
  • 1
    Tempted to down-vote as **This is my code but it's not working** doesn't help us find the problem. But, saying that - `InStr(1, celltxt, "-")` will return the position of `-` in the string, so try using `InStr(1, celltxt, "-")>0` – Darren Bartrup-Cook Apr 26 '17 at 15:00
  • I am sorry, I am new to VBA, it does not send any error but it just don't work, I've read a lot trough internet and can't find a way to fix it. It is supposed to copy cells "BA5:BB36" if there is a dash ("-") or a diagonal ("/") in cell "B14" – Azael Contreras Apr 26 '17 at 15:18
  • I think you meant to say "BA5:BB36" in your latest comment. –  Apr 26 '17 at 15:20

3 Answers3

0

Here's the refactored and fixed version:

Sub TWB_Copy_columns()
    'TWB_Copy_columns Macro

    'Range("B14").Select
    'Selection.End(xlToRight).Select
    'celltxt = Selection.Text

    ' Use explicit references and avoid select. In this case, you will need to
    ' qualify the workbook and sheetname of the range you are using. We can then
    ' directly access the value of that range.

    ' Also, no need to declare a string just to hold onto the value. Directly use the value instead
    With ThisWorkbook.Sheets("Sheetname")
        If InStr(1, .Range("B14").End(xlToRight).value, "-") > 0 Or InStr(1, .Range("B14").End(xlToRight).value, "/") > 0 Then
            .Range("BD5:BB36").value = .Range("BA5:BB36").value
        End If
    End With
End Sub

First, always avoid Select and Activate. In this case, I directly assign the values instead of trying to copy, paste, or select. Any time you see Range("A5").Select; Selection.Value you really need Range("A5").Value. Likewise, never have an unqualified range. Range("A5") is the same as saying ActiveSheet.Range("A5") which can complicate things if the wrong sheet is active.

Finally, if you are literally using a variable for one comparison, use the direct value. There's no need to create a variable for just one task (at least in my opinion).

Edit:

As Ralph suggests, consider reading this thread: How to avoid using Select in Excel VBA macros. Once you learn to avoid Select your abilities will skyrocket.

Community
  • 1
  • 1
Brandon Barney
  • 2,382
  • 1
  • 9
  • 18
  • You might want to reference this post in your answer: http://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba-macros It is quite elaborate on how to avoid using `Select`. Yet, I doubt that this is the real problem here. I am guessing that the OP has "incorrect" code in the sense that it does not do what he / she really wants. – Ralph Apr 26 '17 at 15:05
  • @Ralph Good point. I used that same reference elsewhere, but didnt think of it here. – Brandon Barney Apr 26 '17 at 15:05
0

It seems that you are looking at the displayed format of a cell's contents to determine if it is a date or not. There is a native VBA function, IsDate that does this determination quite well on true dates. If your data does noot contain dates as true dates, well.. they should be true dates so that is another problem to be fixed.

with worksheets("sheet1")
    if isdate(.cells(14, "B").end(xltoright)) then
        .range("BA5:BB36").copy destination:=.range("BD5")
    end if
end with

It seems to me that this code is only reusable if BA5:BB36 is not static but you've provided no indication on what determines the location. It's probably the last two columns of data in your data block but that is only a guess.

0

This does what I think you're looking for (for every "-" or "/", copy Range("BA5:BB36") and paste it to Range("BD5"), Range("BG5") - leaving a space in your columns):

Sub TWB_Copy_columns()
'TWB_Copy_columns Macro
Dim celltxt As String
Dim vWords As Variant
Dim rFind As Range
Dim i As Long

celltxt = Range("B14").Value
celltxt = Replace(celltxt, "-", "/")

vWords = Split(celltxt, "/")

Range("BA5:BB36").Copy
Range("BD5").Activate

For i = 1 To UBound(vWords)
    ActiveCell.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks:=False, Transpose:=False
    ActiveCell.Offset(0, 2).Activate
Next

End Sub
Jeremy
  • 1,337
  • 3
  • 12
  • 26