1
\$\begingroup\$

I have the below code. I have list of status which contain may contain 4 values. R,UR,DO,NDO

based on this list. i need to convert to Y,N values

If list contains R the Readstatus="Y"

If list contains UR the Readstatus="N"

If both the ReadStatus="Y,N"

If list contains DO the DoneStatus="Y"

If list contains NDO the DoneStatus="N" If both the DoneStatus="Y,N"

the below code works but its too ugly. is there any thing i can change

                Dim Readstatus As String = ""
                Dim DoneStatus As String = ""
                If Not String.IsNullOrEmpty(Status) Then
                    Dim listStatus = Status.Split(",").ToList()
                    If listStatus.Contains("R") Then
                        Readstatus = "Y"
                    End If
                    If listStatus.Contains("UR") Then
                        If String.IsNullOrEmpty(Readstatus) Then
                            Readstatus = "N"
                        Else
                            Readstatus = Readstatus & ",N"
                        End If

                    End If
                    If listStatus.Contains("DO") Then
                        DoneStatus = "Y"
                    End If
                    If listStatus.Contains("NDO") Then
                        If String.IsNullOrEmpty(DoneStatus) Then
                            DoneStatus = "N"
                        Else
                            DoneStatus = DoneStatus & ",N"
                        End If
                    End 
\$\endgroup\$
1
\$\begingroup\$

It's interesting to notice that when you parse your input, you break a comma-separated string into a list of values. The end result of your code can be achieved much more cleanly if you take a similar (but inverted) approach, i.e. generate a list of values and then have the comma-separated string generated from that list.

It's counterintuitive for the logical branching in your code to be significantly more complex than the logical branching in your description of the behavior. These should map one to one. You can just translate your pseudocode logic exactly as is.

The reason your code ended up being more complex is because you needed to add the whole "add a comma or don't" logic to it - which can be avoided by relying on String.Join, which performs that logic for you, as long as you supply it with a collection of values.

If list contains R the Readstatus="Y"
If list contains UR the Readstatus="N"

Dim readStatusList As New List(Of String)

If listStatus.Contains("R") Then
    readStatusList.Add("Y")
End If
If listStatus.Contains("UR") Then
    readStatusList.Add("N")
End If

Dim readStatus As String = String.Join(",", readStatusList)

The exact same applies to the "done" status:

If list contains DO the Donestatus="Y"
If list contains NDO the Donestatus="N"

Dim doneStatusList As New List(Of String)

If listStatus.Contains("DO") Then
    doneStatusList.Add("Y")
End If
If listStatus.Contains("NDO") Then
    doneStatusList.Add("N")
End If

Dim doneStatus As String = String.Join(",", doneStatusList)
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Yes that does look much cleaner. and readable thanks \$\endgroup\$ – Avinash Reddy May 5 at 10:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.