+ Reply to Thread
Results 1 to 4 of 4

Goto misused: help to tidy Code

  1. #1
    Forum Contributor
    Join Date
    03-03-2005
    Posts
    315

    Goto misused: help to tidy Code

    I have a Listbox fed by RowSource delivering data from Cols A-C. The code below
    deletes a selected row from both the ListBox and the root row on the worksheet. It works fine except that, try as I would, it does the job uglily. For one, I have violated one of the cardinal principles of good programming by pandering to the use of GO TO in a way which makes the code poorly structured.

    Could someone kindly have a quick study and restructure the logical flow without having to loop backwards the way I did? Many thanks.

    [PS: I would also love the code to allow for multiple row selection and resultant block deletions, if possible].


    David.

    Private Sub CmdDelete_Click()

    Restart:
    If ListBox1.ListIndex = -1 Then 'no selection
    ans = MsgBox("Select item to delete", vbYesNo + vbDefaultButton2)
    If ans = vbYes Then
    ListBox1.Selected(0) = True 'select 1st item for a start
    GoTo Skip
    Else
    ListBox1.ListIndex = -1
    Exit Sub
    End If
    End If

    Skip:
    If ListBox1.Selected(1) =False True Then
    If ListBox1.Selected(ListBox1.ListIndex) = True Then
    ansx = MsgBox("Do you wish to delete selection?" & vbCrLf & " " & ListBox1.List(ListBox1.ListIndex, 0), vbYesNo + vbDefaultButton2 + vbInformation)
    If ansx = vbNo Then Exit Sub
    ActiveSheet.Cells(ListBox1.ListIndex + 1, 1).Resize(, 3).ClearContents
    On Error Resume Next
    ListBox1.Selected(ListBox1.ListIndex) = False
    ansx = MsgBox("Do you wish to delete another?", vbYesNo + vbDefaultButton1 + vbInformation)
    If ansx = vbNo Then
    GoTo Sortt
    Else
    GoTo Restart
    End If
    End If
    End If


    Sortt:
    Columns("a:c").Sort Key1:=Range("A2"), Key2:=Range("b2"), Key3:=Range("c2"), Header:=xlNo

    ListBox1.RowSource = "a1:c" & [a65536].End(xlUp).Row

    End Sub

  2. #2
    Bob Phillips
    Guest

    Re: Goto misused: help to tidy Code

    Private Sub CmdDelete_Click()

    With Listbox1
    Do
    If .ListIndex = -1 Then 'no selection
    ans = MsgBox("Select item to delete", vbYesNo +
    vbDefaultButton2)
    If ans = vbNo Then
    .ListIndex = -1
    Exit Sub
    Else
    .Selected(0) = True 'select 1st item for a start
    End If
    End If

    If .Selected(1) = False Then
    If .Selected(.ListIndex) = True Then
    ansx = MsgBox("Do you wish to delete selection?" &
    vbCrLf & _
    " " & .List(.ListIndex, 0), _
    bYesNo + vbDefaultButton2 + vbInformation)
    If ansx = vbYes Then
    ActiveSheet.Cells(.ListIndex + 1, 1).Resize(,
    3).ClearContents
    On Error Resume Next
    .Selected(.ListIndex) = False
    ansx = MsgBox("Do you wish to delete another?", _
    vbYesNo + vbDefaultButton1 + vbInformation)
    End If
    End If
    End If
    Loop Until ansx = vbNo

    Columns("A:C").Sort Key1:=Range("A2"), _
    Key2:=Range("b2"), _
    Key3:=Range("c2"), _
    Header:=xlNo

    .RowSource = "A1:C" & Cells(Rows.Count, "A").End(xlUp).Row

    End With

    End Sub



    --
    HTH

    Bob Phillips

    (remove nothere from email address if mailing direct)

    "davidm" <[email protected]> wrote in
    message news:[email protected]...
    >
    > I have a Listbox fed by RowSource delivering data from Cols A-C. The
    > code below
    > deletes a selected row from both the ListBox and the root row on the
    > worksheet. It works fine except that, try as I would, it does the job
    > uglily. For one, I have violated one of the cardinal principles of -good
    > programming- by pandering to the use of GO TO in a way which makes the
    > code poorly structured.
    >
    > Could someone kindly have a quick study and restructure the logical
    > flow without having to loop backwards the way I did? Many thanks.
    >
    > [PS: I would also love the code to allow for multiple row selection and
    > resultant block deletions, if possible].
    >
    >
    > David.
    >
    > Private Sub CmdDelete_Click()
    >
    > Restart:
    > If ListBox1.ListIndex = -1 Then 'no selection
    > ans = MsgBox("Select item to delete", vbYesNo + vbDefaultButton2)
    > If ans = vbYes Then
    > ListBox1.Selected(0) = True 'select 1st item for a start
    > GoTo Skip
    > Else
    > ListBox1.ListIndex = -1
    > Exit Sub
    > End If
    > End If
    >
    > Skip:
    > If ListBox1.Selected(1) =False True Then
    > If ListBox1.Selected(ListBox1.ListIndex) = True Then
    > ansx = MsgBox("Do you wish to delete selection?" & vbCrLf & "
    > " & ListBox1.List(ListBox1.ListIndex, 0), vbYesNo +
    > vbDefaultButton2 + vbInformation)
    > If ansx = vbNo Then Exit Sub
    > ActiveSheet.Cells(ListBox1.ListIndex + 1, 1).Resize(,
    > 3).ClearContents
    > On Error Resume Next
    > ListBox1.Selected(ListBox1.ListIndex) = False
    > ansx = MsgBox("Do you wish to delete another?", vbYesNo +
    > vbDefaultButton1 + vbInformation)
    > If ansx = vbNo Then
    > GoTo Sortt
    > Else
    > GoTo Restart
    > End If
    > End If
    > End If
    >
    >
    > Sortt:
    > Columns("a:c").Sort Key1:=Range("A2"), Key2:=Range("b2"),
    > Key3:=Range("c2"), Header:=xlNo
    >
    > ListBox1.RowSource = "a1:c" & [a65536].End(xlUp).Row
    >
    > End Sub
    >
    >
    > --
    > davidm
    > ------------------------------------------------------------------------
    > davidm's Profile:

    http://www.excelforum.com/member.php...o&userid=20645
    > View this thread: http://www.excelforum.com/showthread...hreadid=494441
    >




  3. #3
    Dave Peterson
    Guest

    Re: Goto misused: help to tidy Code

    You have another response to your other post.

    davidm wrote:
    >
    > I have a Listbox fed by RowSource delivering data from Cols A-C. The
    > code below
    > deletes a selected row from both the ListBox and the root row on the
    > worksheet. It works fine except that, try as I would, it does the job
    > uglily. For one, I have violated one of the cardinal principles of -good
    > programming- by pandering to the use of GO TO in a way which makes the
    > code poorly structured.
    >
    > Could someone kindly have a quick study and restructure the logical
    > flow without having to loop backwards the way I did? Many thanks.
    >
    > [PS: I would also love the code to allow for multiple row selection and
    > resultant block deletions, if possible].
    >
    > David.
    >
    > Private Sub CmdDelete_Click()
    >
    > Restart:
    > If ListBox1.ListIndex = -1 Then 'no selection
    > ans = MsgBox("Select item to delete", vbYesNo + vbDefaultButton2)
    > If ans = vbYes Then
    > ListBox1.Selected(0) = True 'select 1st item for a start
    > GoTo Skip
    > Else
    > ListBox1.ListIndex = -1
    > Exit Sub
    > End If
    > End If
    >
    > Skip:
    > If ListBox1.Selected(1) =False True Then
    > If ListBox1.Selected(ListBox1.ListIndex) = True Then
    > ansx = MsgBox("Do you wish to delete selection?" & vbCrLf & "
    > " & ListBox1.List(ListBox1.ListIndex, 0), vbYesNo +
    > vbDefaultButton2 + vbInformation)
    > If ansx = vbNo Then Exit Sub
    > ActiveSheet.Cells(ListBox1.ListIndex + 1, 1).Resize(,
    > 3).ClearContents
    > On Error Resume Next
    > ListBox1.Selected(ListBox1.ListIndex) = False
    > ansx = MsgBox("Do you wish to delete another?", vbYesNo +
    > vbDefaultButton1 + vbInformation)
    > If ansx = vbNo Then
    > GoTo Sortt
    > Else
    > GoTo Restart
    > End If
    > End If
    > End If
    >
    > Sortt:
    > Columns("a:c").Sort Key1:=Range("A2"), Key2:=Range("b2"),
    > Key3:=Range("c2"), Header:=xlNo
    >
    > ListBox1.RowSource = "a1:c" & [a65536].End(xlUp).Row
    >
    > End Sub
    >
    > --
    > davidm
    > ------------------------------------------------------------------------
    > davidm's Profile: http://www.excelforum.com/member.php...o&userid=20645
    > View this thread: http://www.excelforum.com/showthread...hreadid=494441


    --

    Dave Peterson

  4. #4
    Forum Contributor
    Join Date
    11-11-2005
    Posts
    267
    Many thanks Dave for both solutions. The second is transplanted here for consolidation, having mistakenly submitted my first post to a wrong thread ( when I had meant to initiate one). One again, thanks for your assistance.

    David.


    --------------------------------------------------------------------------------
    Dave Paterson wrote:

    A listbox can support multiple selections. Maybe you could use that to get all
    the rows that that should be deleted/cleared.

    I put 2 buttons (cmddelete and cmdcancel) and one listbox (listbox1) on a
    userform.

    This was the code behind that userform:

    Option Explicit
    Dim BlkProc As Boolean
    Private Sub cmdCancel_Click()
    Unload Me
    End Sub
    Private Sub cmdDelete_Click()

    Dim iCtr As Long
    Dim myRng As Range
    Dim RngToClear As Range
    Dim myArea As Range
    Dim resp As Long

    resp = MsgBox(Prompt:="Are you sure?", Buttons:=vbYesNo)

    If resp = vbNo Then
    Exit Sub
    End If

    Set RngToClear = Nothing

    With Me.ListBox1
    For iCtr = .ListCount - 1 To 0 Step -1
    If .Selected(iCtr) Then
    If RngToClear Is Nothing Then
    Set RngToClear _
    = Application.Range(.RowSource).Rows(iCtr + 1).Cells(1)
    Else
    Set RngToClear = Union(RngToClear, _
    Application.Range(.RowSource).Rows(iCtr + 1).Cells(1))
    End If
    End If
    Next iCtr
    End With

    If RngToClear Is Nothing Then
    'do nothing
    Else
    For Each myArea In RngToClear.Areas
    myArea.Resize(, 3).ClearContents
    Next myArea
    With Worksheets("Sheet1")
    With .Range("a:c")
    .Cells.Sort key1:=.Columns(1), order1:=xlascending, _
    key2:=.Columns(2), order2:=xlascending, _
    key3:=.Columns(3), order3:=xlascending, _
    header:=xlNo
    Set myRng _
    = .Range("a1:C" & .Cells(.Rows.Count, "A").End(xlUp).Row)
    End With
    End With
    Me.cmdDelete.Enabled = False
    If Application.CountA(myRng) = 0 Then
    'no more data
    Me.ListBox1.RowSource = ""
    Me.ListBox1.Clear
    Else
    Me.ListBox1.RowSource = myRng.Address(external:=True)
    End If

    End If

    End Sub

    Private Sub ListBox1_Change()
    Dim iCtr As Long

    If BlkProc = True Then Exit Sub

    Me.cmdDelete.Enabled = False

    With Me.ListBox1
    For iCtr = 0 To .ListCount
    If .Selected(iCtr) Then
    Me.cmdDelete.Enabled = True
    Exit For
    End If
    Next iCtr
    End With

    End Sub

    Private Sub UserForm_Initialize()
    Dim myRng As Range

    With Worksheets("Sheet1")
    Set myRng = .Range("a1:C" & .Cells(.Rows.Count, "A").End(xlUp).Row)
    End With

    If Application.CountA(myRng) = 0 Then
    'do nothing
    Else
    With Me.ListBox1
    BlkProc = True
    .MultiSelect = fmMultiSelectMulti
    .ColumnCount = 3
    .RowSource = myRng.Address(external:=True)
    BlkProc = False
    End With
    End If

    Me.cmdDelete.Enabled = False
    Me.cmdCancel.Caption = "Cancel"
    Me.cmdDelete.Caption = "Delete"

    End Sub

    davidm wrote:
    >
    > I have a Listbox fed by RowSource delivering data from Cols A-C. The
    > code below
    > deletes a selected row from both the ListBox and the root row on the
    > worksheet. It works fine except that, try as I would, it does the job
    > uglily. For one, I have violated one of the cardinal principles of -good
    > programming- by pandering to the use of GO TO in a way which makes the
    > code poorly structured.
    >
    > Could someone kindly have a quick study and restructure the logical
    > flow without having to loop backwards the way I did? Many thanks.
    >
    > [PS: I would also love the code to allow for multiple row selection and
    > resultant block deletions, if possible].
    >
    > David.
    >
    > Private Sub CmdDelete_Click()
    >
    > Restart:
    > If ListBox1.ListIndex = -1 Then 'no selection
    > ans = MsgBox("Select item to delete", vbYesNo + vbDefaultButton2)
    > If ans = vbYes Then
    > ListBox1.Selected(0) = True 'select 1st item for a start
    > GoTo Skip
    > Else
    > ListBox1.ListIndex = -1
    > Exit Sub
    > End If
    > End If
    >
    > Skip:
    > If ListBox1.Selected(1) =False True Then
    > If ListBox1.Selected(ListBox1.ListIndex) = True Then
    > ansx = MsgBox
    ("Do you wish to delete selection?" & vbCrLf & "
    > " & ListBox1.List(ListBox1.ListIndex, 0), vbYesNo +
    > vbDefaultButton2 + vbInformation)
    > If ansx = vbNo Then Exit Sub
    > ActiveSheet.Cells(ListBox1.ListIndex + 1, 1).Resize(,
    > 3).ClearContents
    > On Error Resume Next
    > ListBox1.Selected(ListBox1.ListIndex) = False
    > ansx = MsgBox("Do you wish to delete another?", vbYesNo +
    > vbDefaultButton1 + vbInformation)
    > If ansx = vbNo Then
    > GoTo Sortt
    > Else
    > GoTo Restart
    > End If
    > End If
    > End If
    >
    > Sortt:
    > Columns("a:c").Sort Key1:=Range("A2"), Key2:=Range("b2"),
    > Key3:=Range("c2"), Header:=xlNo
    >
    > ListBox1.RowSource = "a1:c" & [a65536].End(xlUp).Row
    >
    > End Sub
    >
    > --
    > davidm
    > ------------------------------------------------------------------------
    > davidm's Profile: http://www.excelforum.com/member.php...o&userid=20645
    > View this thread: http://www.excelforum.com/showthread...hreadid=494434

    --

    Dave Peterson

+ Reply to Thread

Thread Information

Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)

Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts

Search Engine Friendly URLs by vBSEO 3.6.0 RC 1