Linq Filter Expression String Builder

[jwcoleman87]

So, I finally managed to slap this bit of code together. I am using it to filter my Dataset. I simply want to show you the code and humbly ask if there are any suggestions to clean it up.

First off, the individual expressions:

    Private Function TechnicianFilter() As String
        Dim LogicalOperator As String = " OR "
        Dim TechFilter As String = ""
        Dim FilterString As String = ""
        If clbTechnicians.CheckedItems.Count <= 1 Then
            LogicalOperator = ""
        End If
        Dim index As Integer = 0
        For Each itemChecked In clbTechnicians.CheckedItems

            If index = clbTechnicians.CheckedItems.Count - 1 Then
                LogicalOperator = ""
            End If
            TechFilter = String.Format("TechnicianID = '{0}' {1}", itemChecked.ToString(), LogicalOperator)
            FilterString += TechFilter
            index += 1
        Next
        If FilterString <> "" Then
            FilterString = "(" + FilterString + ")"
        End If
        Return FilterString
    End Function

    Private Function StatusFilter() As String
        Dim LogicalOperator As String = " OR "
        Dim StatFilter As String = ""
        Dim FilterString As String = ""
        If clbStatus.CheckedItems.Count <= 1 Then
            LogicalOperator = ""
        End If
        Dim index As Integer = 0
        For Each itemChecked In clbStatus.CheckedItems

            If index = clbStatus.CheckedItems.Count - 1 Then
                LogicalOperator = ""
            End If
            StatFilter = String.Format("Status = '{0}' {1}", itemChecked.ToString(), LogicalOperator)
            FilterString += StatFilter
            index += 1
        Next
        If FilterString <> "" Then
            FilterString = "(" + FilterString + ")"
        End If
        Return FilterString
    End Function

    Private Function StartTimeFilter() As String
        Dim FilterString As String = ""

        StartTimeFilter = String.Format("StartTime > '#{0}#' AND StartTime < '#{1}#' AND " & _
                                        "StartTime > '#{2}#' AND StartTime < '#{3}#'", dtpStartTime1.Value.Date,
                                        dtpStartTime1.Value.Date().AddDays(nmMultipleStartDays.Value), dtpStartHour.Value.Hour,
                                        dtpStartHour.Value.AddHours(nmStartHour2.Value))
        FilterString += StartTimeFilter
        Return FilterString
    End Function

    Private Function EndTimeFilter() As String
        Dim FilterString As String = ""
        EndTimeFilter = String.Format("EndTime > '#{0}#' AND EndTime < '#{1}#' AND " & _
                                      "EndTime > '#{2}#' AND EndTime < '#{3}#'", dtpEndTime1.Value.Date(),
                                        dtpEndTime1.Value.Date().AddDays(nmMultipleStartDays.Value), dtpEndHour.Value.Hour,
                                        dtpEndHour.Value.AddHours(nmEndHour2.Value))
        FilterString += EndTimeFilter
        Return FilterString
    End Function

So there are 4 individual filters, one for technicians, one for unit status, one for start times, and one for end times. So far these all work, (to the best of my knowledge, as I wrote the hour part at home, I'll get to test that tomorrow)
So these four functions are slapped together into a string sandwich in this function:

    Private Sub btnApplyFilter_Click(sender As Object, e As EventArgs) Handles btnApplyFilter.Click
        Dim TechFilter As String = TechnicianFilter()
        Dim StatFilter As String = StatusFilter()
        Dim StartFilter As String = StartTimeFilter()
        Dim EndFilter As String = EndTimeFilter()
        Dim LogicalOperator0 As String = " AND "
        Dim LogicalOperator1 As String = " AND "
        Dim LogicalOperator2 As String = " AND "

        'conditional statement for item {1} of filter expression
        If TechFilter = "" Or StatFilter = "" Then
            LogicalOperator0 = ""
        End If
        'conditional statement for item {3} of filter expression
        If (TechFilter = "" And StatFilter = "") Or StartFilter = "" Then
            LogicalOperator1 = ""
        End If
        'conditional statement for item {5} of filter expression
        If (TechFilter = "" And StatFilter = "" And StartFilter = "") Or EndFilter = "" Then
            LogicalOperator2 = ""
        End If
        Dim FilterExpression As String = String.Format("{0}{1}{2}{3}{4}{5}{6}", TechFilter, LogicalOperator0, StatFilter,
                                                       LogicalOperator1, StartFilter, LogicalOperator2, EndFilter)
        txtFilterString.Text = FilterExpression
        RepairBindingSource.Filter = txtFilterString.Text
    End Sub

So ultimately I am wondering if there is any good way to clean all this code up, polish it and make it neat. I was wondering about the logicaloperator strings, and how I might be more efficient.

Ideas, criticisms, comments, questions welcome!

 

[jmcilhinney]

I haven't had a thorough look at that first snippet, although I'm curious why you have single-quotes and hash marks around the date/time parts. As for the second snippet, you don't need any conditional statements:

    Dim TechFilter As String = TechnicianFilter()
    Dim StatFilter As String = StatusFilter()
    Dim StartFilter As String = StartTimeFilter()
    Dim EndFilter As String = EndTimeFilter()
    Dim FilterExpression As String = String.Join(" AND ", {TechFilter, StatFilter, StartFilter, EndFilter}.Where(Function(s) s <> String.Empty))

The Where call excludes any empty filters and the Join call places the operator between each pair of filters.

 

[jwcoleman87]

Thank you for your question and for your suggestions, both have been implemented:

    Private Sub btnApplyFilter_Click(sender As Object, e As EventArgs) Handles btnApplyFilter.Click
        Dim TechFilter As String = TechnicianFilter()
        Dim StatFilter As String = StatusFilter()
        Dim StartFilter As String = StartTimeFilter()
        Dim EndFilter As String = EndTimeFilter()
        Dim SerialFilter As String = SerialNumberFilter()
        Dim FilterExpression As String = String.Join(" AND ", {TechFilter, StatFilter, StartFilter, EndFilter, SerialFilter}.Where(Function(s) s <> String.Empty))
        dgvRepairs.DataSource = RepairBindingSource
        RepairBindingSource.Filter = FilterExpression
    End Sub

This is much neater! I think I could actually get rid of my local strings and pass the functions as part of the paramarray.

 

[jmcilhinney]

I think I could actually get rid of my local strings and pass the functions as part of the paramarray.

As long as you don't need to use the same values again soon after, you certainly could remove the variables altogether.

I would also tend to change those method names. A simple rule to follow is that fields and properties represent data and should therefore be noun-oriented while methods represent behaviour and should therefore be verb-oriented. A property might be named TechnicianFilter but a method should be named something like GetTechicianFilter.

 

[Herman]

Just as a follow up to the other thread about Linq you started, if you decide to go with Linq to SQL then the following replaces your first filter entirely:

Dim techs = From t In db.Technicians
            Join ct In clbTechnicians.CheckedItems On t.TechName = ct.ToString
            Select t

Of course you still need to instantiate your db data context and encase everything into a property or otherwise. After this techs will be an IEnumerable(Of Technician) containing only the techs you checked.

 

[jwcoleman87]

Just as a follow up to the other thread about Linq you started, if you decide to go with Linq to SQL then the following replaces your first filter entirely:

Dim techs = From t In db.Technicians
            Join ct In clbTechnicians.CheckedItems On t.TechName = ct.ToString
            Select t

Of course you still need to instantiate your db data context and encase everything into a property or otherwise. After this techs will be an IEnumerable(Of Technician) containing only the techs you checked.

WHAT, you just joined a non-db object with a DB object. WHAT WITCHCRAFT IS THIS?!

As long as you don't need to use the same values again soon after, you certainly could remove the variables altogether.

I would also tend to change those method names. A simple rule to follow is that fields and properties represent data and should therefore be noun-oriented while methods represent behaviour and should therefore be verb-oriented. A property might be named TechnicianFilter but a method should be named something like GetTechicianFilter.

JM -- I will take your recommendation to heart, as I am a young padowan in the art of programming good practices.

 

[Herman]

You are right you pointed out a problem here, cross-context joins, but this is easily resolved:

' Enumerate the techs names from the database, filter it to get only the items in the checked items list.
Dim techs = db.Technicians.Where(Function(t) clbTechnicians.CheckedItems.Contains(t.TechName))

 

[jwcoleman87]

I think this works to get a list of the technicians.

            Dim Technames = From t In db.Users
                            Select t.UserID.ToUpper.Distinct
            For Each value As String In Technames
                clbTechnicians.Items.Add(value, False)
            Next

That didn't work....

But I think this will:

            Dim Technames = From t In db.Users.AsEnumerable _
                            Select t.UserID.ToUpper.Distinct

            clbTechnicians.Items.AddRange(Technames.ToArray())

Close, but this is the real deal, finally got it!

            Dim Technames As Array = (
                From r In db.Users.ToArray()
                Select r.UserID.ToUpper Distinct).ToArray()
            clbTechnicians.Items.AddRange(Technames)

This sets up my technician filter list based off of what names are actually in the db!

 

[JohnH]

select string.ToUpper.Distinct

select string.ToUpper Distinct

Beware the dot! It directly connects two expressions. The first part I quoted from your code is actually returning the distinct upper chars for each field value, because a string is also a Char array (enumerable). The second code correctly returns the upper case fields that is distinct.

There is also another significant difference, first code is using Linq expression Distinct, the second code is using Enumerable.Distinct generic function. If you wanted to use the function, f.ex because you needed a custom comparer, then you would have to first enclose Linq selection in parantheses, before calling it like this:

(select string.ToUpper).Distinct