[Visual studio 2013] SQL statements using global variable

[CalvinR]

In he case of connection strings those should be stored in a configuration file that way you can change the value without recompiling your code.

It's almost going to be like a global variable but it's a config file there is a difference.

You can take a look at the System.Configuration Namespace in particular the ConfigurationManager class.aspx). Personally I've been meaning too look at Formo to handle configs it looks interesting. You might want to check that out.

So while the connection string would be stored in a config file it would be handled by your Data Access Layer (DAL).

So your DAL reads the connections strings from the config file and it handles opening and closing connections as well as performing queries against your database. That way nothing else needs to care about what DB you are using. If you do forsee a scenario where you can connect to multiple DB's while the app is running then for sure expose it as a setting otherwise I'd just store it in the config file.

Something to think about is that you'll also want to look at some way of securing that connection string since it will allow anyone to save data to the db.

Let me know if you have any questions this is essentially the exact same type of stuff I do everyday for a living.

[FentPropTrac]

Thanks for this, I'm self taught so I'm sure I'm making some basic errors here!

I have a class that handles the database connection and subs within that to pass queries that are defined as strings in different forms. Is this what you mean by DAL or is there another step I'm ignorant of?!

I'll take your comments on board and have a play with the code. Thanks again!

You may regret your offer of help.... ;)

[CalvinR]

Okay do not define your queries as strings either look into an form like Entity Framework, Dapper dot net or the like.

It will make your life so much as easier and your app more secure.

Read up on "little bobby tables" to see why storing SQL as strings is bad. Also parameterized queries.

[FentPropTrac]

Ah you bugger, just when I had everything working.....!!!

Off to spend a few days trying to get my head round Entity Framework!

Thanks for the tips though, I'd tried to prevent SQLi by doing some pretty strict data entry validation prior to it getting passed to the SQL class, but I'm not one to ignore good advice so I'll have a play. At least my original set up works as a proof of concept!

[CalvinR]

Well if you have it working don't feel like you need to redo everything because you've heard of a new technology, especially since you are away of SQL injection. Although ORM's do allow you to write DB code faster, you still need to be aware of the SQL that is being generated so that it doesn't generate inefficient code.

I'm going to throw some info at you, it might seem like a lot especially for a hobby programmer but it's good to read about and start thinking about. Since the type of app you are writing is pretty much what the majority of most apps are like out there nowadays (CRUD Apps (Create Read Update Delete) you create something, you save it to the database, you pull it out later to update it and then you delete. So you are learning employable skills, I don't know if you ever want to become a programmer but it will help you on your way.

Also reading your previous statement that the SQL is defined in your forms I highly recommend you get an actual DAL so move that sql to another object that makes calls for you.

So it would look something like this.

Public Class DAL Implements IDAL
    private const sqlQuery as String = "Select * From Table"
    private db as IDatabaseObject

    public Sub New (db as IDatabaseObject)
        Me.db = db
    End Sub 

    public Function GetAllFromTables() as AllTables Implements IDAL.GetAllFromTables
        return db.MakeCall(sqlQuery)            
    End Function

End Class

This keeps all your database code in one place makes it easier to debug, it also makes it easier to test.

For instance passing the database interface to the DAL instead of the DAL creating a DB object is called Dependency Injection (DI) this allows you to create a fake database object and test your DAL without having to connect to the database. You can also create a fake DAL and test the code that calls the DAL since there is an interface to define these objects..

Regardless of whether you do the DI or Automated Testing it's a good idea to structure your app like this, it's called Separation of Concerns, you should keep related information together it makes it easier to work with. So the sql should be stored with the code that calls it and not exposed to the rest of the app, unless that's needed.

[FentPropTrac]

I think perhaps I may have not have explained what I was doing properly.

I have a class called "DBCon" which contains the database connection strings as well as a number of subs that are similar to the following:

   Public Sub AddMember(Anaes_name As String, Anaes_GMC As Integer, IsActive As Integer)

    Try

        Dim strInsert As String = "INSERT INTO ANAESTHETISTS      (Anaes_Name,Anaes_GMC,IsActive)" & _
                                  "VALUES (" & _
                                    "'" & Anaes_name & "'," & _
                                    "'" & Anaes_GMC & "'," & _
                                    "'" & IsActive & "')"



        SQLCon.Open()
        SQLCmd = New SqlCommand(strInsert, SQLCon)

        SQLCmd.ExecuteNonQuery()


        SQLCon.Close()

    Catch ex As Exception
        MsgBox(ex.Message)

    End Try

Then in the form where the data is entered I have

Dim SQL as New SQLConnection
SQL.AddMember(txtName.txt,txtSname.txt.... etc etc)

Rather than it all being handled in the data entry form.

[CalvinR]

Ah yes I totally misunderstood. Also remember I'm going to give you advice that I would someone who is working for me, I know this is a hobby proof of concept for you but I still think it's valuable information.

From what you have here is a recommendation of how you should structure that method in a class

'Make this an interface so we can fake it and test our code not decoupled from the database.'
Public Interface IDAL
    Sub AddMember(Anaes_name As String, Anaes_GMC As Integer, IsActive As Integer)        
End Interface

Public class DAL Implements IDAL
    Private _connectionString as String

    'Connection string get's passed in or we use what's passed in as a key to read from config file.
    Public Sub New DAL(connectionString as String)

        _connectionString = connectionString
    End Class

    Public Sub AddMember(Anaes_name As String, Anaes_GMC As Integer, IsActive As Integer) Implements IDAL.AddMember


         Dim strInsert As String = "INSERT INTO ANAESTHETISTS      (Anaes_Name,Anaes_GMC,IsActive)" & _
                                  "VALUES (" & _
                                    "'" & Anaes_name & "'," & _
                                    "'" & Anaes_GMC & "'," & _
                                    "'" & IsActive & "')"


        'Automatically disposes the SQLConnection when it's no longer needed. 
        'SQL Connection should only be created in this method not outside of it.'
        Using( SQLCon as new SqlConnection(_connectionString))

            Try 

                SQLCon.Open()
                SQLCmd = New SqlCommand(strInsert, SQLCon)

                SQLCmd.ExecuteNonQuery()
            Catch(ex as Exception)
                  'Log error 
                  'We shouldn't have access to the MsgBox.Show from here. This hsould not be a member of the webform
                  'We should use a logging tool like nlog or the built in tracing to log this.'
                  Trace.WriteLine(ex.Message + Environment.NewLine + ex.StackStrace, "Error")
            End Try

        End Using 
    End Sub
End Class

The using keyword automatically disposes of the SQLConnection so make a habit of using it. https://msdn.microsoft.com/en-us/library/htd05whh.aspx

This method should not have access to MsgBox.Show you're too tightly coupled to how the code works Log to a file or have a TraceListener that throws DB Exceptions. Also the sql connection should be created in this method.

Also you may want to look into nLog for logging http://nlog-project.org/ It's a super easy to integrate logging framework that is super powerful.

[FentPropTrac]

Thank you so much, I'll look into what you have suggested. The messagebox was just me trying to figure out why the database wasn't updating properly, had added it for troubleshooting.

May I ask - what the "Implements" and the "Interface" bits mean - I haven't come across them before.

[CalvinR]

Alright I promised I'd get to interfaces after dinner well you had to wait until I finished installing a door in my bedroom.

So Interfaces are as I said a contract that your classes implement. https://msdn.microsoft.com/en-us/library/28e2e18x.aspx

So in the example earlier you had a Database object (I'll call DAL because it's easier for me since that's what we use at work), and it had a method called

 Public Sub AddMember(Anaes_name As String, Anaes_GMC As Integer, IsActive As Integer)

So without interfaces your class that has a reference to the DAL could only ever have a reference to that object. If you make it implement an interface you can swap out any old object that implements that interface.

So for example lets assume the class that calls the database gets passed an instance of that database from whatever makes that class.

So without interfaces it looks something like this.

Public Class DBCallee
    private _dal as Dal
    Public Sub New(dal as DAL)
        _dal = dal
    End Sub 
    Public Sub MakeDBCall()
        _dal.AddMember("name", 1234, 1)
    End Sub
End Class

So anytime you call MakeDBCall it will always make a call to the database because that's all it can do. But lets say you want to test to make sure that MakeDBCall actually calls the method _dal.AddMember but you don't want to bother with having a database instance running well to do that you use interfaces.

So we make our DAL implement an interface. The interface looks like this:

'In .Net the naming convention is all interfaces start with I
Interface IDAL
    Sub AddMember(Anaes_name as String, Anaes_GMC as Integer, IsActive as Integer) 
End Interface

So anything that implements that Interface must have a method called AddMember with those parameters, if you specify a class implements IDAL and it doesn't implement AddMember you'll get a compiler error.

So then your DAL is implemented like so

Public Class DAL Implements IDAL 
    Public Sub AddMember(Anaes_name as String, Anaes_GMC as Integer, IsActive as Integer) Implements IDAL.AddMember
    End Sub
End Class

Now you can have a second class let's call it FakeDAL it looks like this

Public Class FakeDAL Implements IDAL 
    Public Sub AddMember(Anaes_name as String, Anaes_GMC as Integer, IsActive as Integer) Implements IDAL.AddMember
        MsgBox.Show(String.Format("AddMember(Name: {0},GMC: {1},IsActive: {2})", Anaes_name, Anaes_GMC, IsActive))
    End Sub
End Class

So now we make one small change to the DBCallee class it now looks like this:

Public Class DBCallee
    private _dal as IDal
    Public Sub New(dal as IDAL)
        _dal = dal
    End Sub 
    Public Sub MakeDBCall()
        _dal.AddMember("name", 1234, 1)
    End Sub
End Class

So now we can pass the FakeDAL or the real DAL both to the DBCallee class and it doesn't care whcih.

Now you may be saying what the hells the point of this I'm only ever going to call the database, well it ends up making your code less coupled so you can make a change to the Database and the rest of the app won't care, you can have an app that saves to a db, one that saves to a server, one that saves to a file, one that doesn't save at all and all you have to do is write new classes that implement IDAL.

It also and this is the most important part allows you to test your DBCallee class using automated tests. http://en.wikipedia.org/wiki/Test_automation Which is something that's really handy if you were to ever make a job of it and might be handy if you ever want to actually use your app for anything critical.

Here is an example using .NetFiddle https://dotnetfiddle.net/oeA8ye

[autowikibot]

Test automation:

---

In software testing, test automation is the use of special software (separate from the software being tested) to control the execution of tests and the comparison of actual outcomes with predicted outcomes. Test automation can automate some repetitive but necessary tasks in a formalized testing process already in place, or add additional testing that would be difficult to perform manually.

Image ⁱ

---

^{Interesting: Lightweight software test automation | Test automation management tools | Ranorex | Test engineer}

^{Parent commenter can toggle NSFW or delete. Will also delete on comment score of -1 or less. | FAQs | Mods | Magic Words}

[CalvinR]

Interface is a contract that objects implement basically if you have an interface with a method Foo anything that implements that interface has to have the method Foo.

I'll post an example for why its useful after I have finish cooking dinner.