1

I have a string that has FTP permissions - "LRSDCWAN" Is there a more efficiant way of checking the relevent CheckBox if the string contains the relevant character?

 If reader.Item("home_perm").Contains("L") Then CBoxList.Checked = True End If If reader.Item("home_perm").Contains("R") Then CBoxRead.Checked = True End If If reader.Item("home_perm").Contains("S") Then CBoxSubDir.Checked = True End If If reader.Item("home_perm").Contains("D") Then CBoxDelete.Checked = True End If If reader.Item("home_perm").Contains("C") Then CBoxCreate.Checked = True End If If reader.Item("home_perm").Contains("W") Then CBoxWrite.Checked = True End If If reader.Item("home_perm").Contains("A") Then CBoxAppend.Checked = True End If If reader.Item("home_perm").Contains("N") Then CBoxRename.Checked = True End If

Thanks.

Improve this question

5 Answers 5

Reset to default
5

While it doesn't get rid of your .Contains() problem, you can simplify the logic quite a bit.

If you notice, you are using:

If reader.Item("home_perm").Contains("L") Then CBoxList.Checked = True End If

You can simplify this by just saying

CBoxList.Checked = reader.Item("home_perm").Contains("L")

You can do this for all of your checkboxes. It doesn't solve the needing to call contains, but it eliminates 2/3 of your lines of code.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

Big maybe here ... if this is appropriate, this will also fix a bug that may exist with the OP's code, by unchecking items which aren't in the permission list. If though, OP is intentionally avoiding unchecking items, this will cause a bug.
2

EDIT.. Doh, I missed the fact that it's a different checkbox for each character.

Okay, in that case I'd use a Dictionary(Of Char, CheckBox) for each character. Something like this - but in less broken VB :)

' Assumes VB10 collection initializers Dim map As New Dictionary(Of Char, CheckBox) From { { "L", CBoxList }, { "R", CBoxRead }, { "S", CBoxSubDir }, { "D", CBoxDelete }, { "C", CBoxCreate } } For Each c As Char In reader.Item("home_perm") Dim cb As CheckBox If map.TryGetValue(c, cb) Then cb.Checked = True End If Next
Improve this answer

2 Comments

I think you overlooked that there is a separate checkbox for each value
Doh - didn't spot that. How annoying.
2

Regular Expressions.

http://msdn.microsoft.com/en-us/library/hs600312(VS.71).aspx

OR

string.indexof method:

 Dim myString As String = "LRSDCW" Dim myInteger As Integer myInteger = myString.IndexOf("D") // myInteger = 4 myInteger = myString.IndexOf("N") // myInteger = -1

Use an Array for myInteger and check each member of the array for a value other than -1, if it is -1, don't check the box.

Improve this answer

2 Comments

maybe not, its just my first thought for checking strings, alternatively, you can you the string.indexof method to get the position in the string of a letter, it will give -1 if it doesn't exits, its not elegant, but it will work
MaQleod: I don't think checking for containment of a single character is a problem here; Contains works fine. The problem is the repetition. I don't see how using regular expressions or IndexOf helps there.
0

Could you put all your comparision values in a generic list and walk the list?

Improve this answer

Comments

0 
CBoxRename.Checked = (New Regex("[LRSDCWAN]").Match(reader.Item("home_perm")).Count > 0) ?

This is pretty inefficient - it's a brand new instance of a relatively intensive class every time, so you could cache one and reuse it if you wanted, but this way it fits neatly into one line.

Improve this answer

1 Comment

Curses. Made the same mistake as the legendary Jon Skeet; so I don't feel too bad. You can concatenate the same logic you've used in the form Checkbox.Checked = reader.Item("home_perm").Contains("L")

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.