Goto misused: help to tidy Code

D

davidm

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
 
B

Bob Phillips

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 said:
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
Click to expand...
 
D

Dave Peterson

You have another response to your other post.
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
Click to expand...
 
M

Myles

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
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:
Click to expand...
http://www.excelforum.com/showthread...hreadid=494434
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

move item in multiselect listbox 1
Hidden sheets in dialog 1
Offset problem 5
ListBox with header row, delete row, protect header row 2
listbox click event help 4
Need Help Creating a Loop 4
what is the code cann make this pls. help me 1
move item from one listbox to another listbox 6

Top