patternsqlMinor
User Search SQL Builder
Viewed 0 times
sqlusersearchbuilder
Problem
Using the VS2012 code analyzer i see the following code has a pretty low Maintainability Index(40) but i cant seem to think of a better way to design it. Does anyone have suggestions on cleaning this up?
This code is part of a search screen for a web app I'm developing. They have several text fields they can use as well as some select lists. Is there any way to simplify this or make it cleaner? I'm more interested in cleaning the VB up but if someone notices my sql has issues, im open for that too =-)
Edit:
As some commentators correctly pointed out, this code doesn't sanitize the input. Most of the time I use AddInParameter but since this is a bit more dynamic, i decided to sanitize in the BLL before even calling this function. I figured it would help keep this DAL code cleaner if i did it in the BLL instead
```
Shared Function SearchSession(searchCriteria As SessionSearchCriteria) As List(Of Sessions_BO)
Dim Result As New List(Of Sessions_BO)
Dim DatabaseObject As Database = DatabaseFactory.CreateDatabase(SQLUtils.GetConnectionString)
Dim SqlString = .Value
If (Not String.IsNullOrEmpty(searchCriteria.SearchSessionTitle)) _
Or (Not String.IsNullOrEmpty(searchCriteria.SearchSessionDateFrom)) _
Or (Not String.IsNullOrEmpty(searchCriteria.SearchSessionDateTo)) _
Or (searchCriteria.SessionType <> -1) _
Or (searchCriteria.SessionAuthor <> -1) _
Or (Not searchCriteria.SessionSearchKeywords Is Nothing) _
Or (Not searchCriteria.TargetLearners Is Nothing) _
Or (searchCriteria.SessionCurriculum <> -1) Then
Dim whereStr As String = BuildWhereClause(searchCriteria)
SqlString &= If(whereStr.Length > 0, " WHERE " & whereStr, "")
End If
Try
Using DatabaseCommand = DatabaseObject.GetSqlStringCommand(SqlString)
Using DataReader As IDataReader = DatabaseObject.ExecuteReader(DatabaseCommand)
While DataReader.Read()
Result.Add
This code is part of a search screen for a web app I'm developing. They have several text fields they can use as well as some select lists. Is there any way to simplify this or make it cleaner? I'm more interested in cleaning the VB up but if someone notices my sql has issues, im open for that too =-)
Edit:
As some commentators correctly pointed out, this code doesn't sanitize the input. Most of the time I use AddInParameter but since this is a bit more dynamic, i decided to sanitize in the BLL before even calling this function. I figured it would help keep this DAL code cleaner if i did it in the BLL instead
```
Shared Function SearchSession(searchCriteria As SessionSearchCriteria) As List(Of Sessions_BO)
Dim Result As New List(Of Sessions_BO)
Dim DatabaseObject As Database = DatabaseFactory.CreateDatabase(SQLUtils.GetConnectionString)
Dim SqlString = .Value
If (Not String.IsNullOrEmpty(searchCriteria.SearchSessionTitle)) _
Or (Not String.IsNullOrEmpty(searchCriteria.SearchSessionDateFrom)) _
Or (Not String.IsNullOrEmpty(searchCriteria.SearchSessionDateTo)) _
Or (searchCriteria.SessionType <> -1) _
Or (searchCriteria.SessionAuthor <> -1) _
Or (Not searchCriteria.SessionSearchKeywords Is Nothing) _
Or (Not searchCriteria.TargetLearners Is Nothing) _
Or (searchCriteria.SessionCurriculum <> -1) Then
Dim whereStr As String = BuildWhereClause(searchCriteria)
SqlString &= If(whereStr.Length > 0, " WHERE " & whereStr, "")
End If
Try
Using DatabaseCommand = DatabaseObject.GetSqlStringCommand(SqlString)
Using DataReader As IDataReader = DatabaseObject.ExecuteReader(DatabaseCommand)
While DataReader.Read()
Result.Add
Solution
You are getting a "low Maintainability Index(40)" because you have no separation of concerns here.
Your DAL and SQL Server queries are two totally different things, and then you speak of your Business Logic Layer.
You should never create an entire query string in C#, VB, Python, C, C++, Java, PHP, or any other programming language that you can think of, it is not practical and removes responsibility from the Database.
Here is what you should do
From what I can tell, you will need several Stored Procedures, and doing this will make maintenance of the application much easier.
Your DAL and SQL Server queries are two totally different things, and then you speak of your Business Logic Layer.
You should never create an entire query string in C#, VB, Python, C, C++, Java, PHP, or any other programming language that you can think of, it is not practical and removes responsibility from the Database.
Here is what you should do
- Create a Stored Procedure on the Database.
- Gather and Sanitize Input
- Run the Input through your Business Logic Layer
- Send the Input to the DAL
- Create a class that bundles the information and sends it to the Database via SQLCommand, SQLConnection, and other SQL classes
From what I can tell, you will need several Stored Procedures, and doing this will make maintenance of the application much easier.
Context
StackExchange Code Review Q#54436, answer score: 7
Revisions (0)
No revisions yet.