Skip to main content
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

Is this secure? Well, nothing is ever really secure, but yes. Generally speaking it is. You're using parameterized queries, which is good. Malachi is right though, stored procedures would be even better.Malachi is right though, stored procedures would be even better. What you can do to make this a little more secure is specify the datatype and size of each parameter.

I'm not sure where this code resides, but you should make sure that it is separated from application logic in its own Data Access Layer. You open and close a lot of connections to your back end. That's a performance hit. Implementing a proper DAL should help mitigate that.

What does "3" mean in this piece of code? Nix the magic number and create a constant with a meaningful name.

        For i = 0 To 3
            If thisReader.GetValue(i).ToString() = "" Then
                Throw New SQLException("There has been an error with the database query.")
            End If
        Next

The elephant in the room...

Access is not a great back end for a website. It has notorious performance issues. Considering you're concerned about security as well, I would recommend moving to a SQL Server Express backend. You'll find the move relatively painless and have a faster, more secure database in the long run.

Is this secure? Well, nothing is ever really secure, but yes. Generally speaking it is. You're using parameterized queries, which is good. Malachi is right though, stored procedures would be even better. What you can do to make this a little more secure is specify the datatype and size of each parameter.

I'm not sure where this code resides, but you should make sure that it is separated from application logic in its own Data Access Layer. You open and close a lot of connections to your back end. That's a performance hit. Implementing a proper DAL should help mitigate that.

What does "3" mean in this piece of code? Nix the magic number and create a constant with a meaningful name.

        For i = 0 To 3
            If thisReader.GetValue(i).ToString() = "" Then
                Throw New SQLException("There has been an error with the database query.")
            End If
        Next

The elephant in the room...

Access is not a great back end for a website. It has notorious performance issues. Considering you're concerned about security as well, I would recommend moving to a SQL Server Express backend. You'll find the move relatively painless and have a faster, more secure database in the long run.

Is this secure? Well, nothing is ever really secure, but yes. Generally speaking it is. You're using parameterized queries, which is good. Malachi is right though, stored procedures would be even better. What you can do to make this a little more secure is specify the datatype and size of each parameter.

I'm not sure where this code resides, but you should make sure that it is separated from application logic in its own Data Access Layer. You open and close a lot of connections to your back end. That's a performance hit. Implementing a proper DAL should help mitigate that.

What does "3" mean in this piece of code? Nix the magic number and create a constant with a meaningful name.

        For i = 0 To 3
            If thisReader.GetValue(i).ToString() = "" Then
                Throw New SQLException("There has been an error with the database query.")
            End If
        Next

The elephant in the room...

Access is not a great back end for a website. It has notorious performance issues. Considering you're concerned about security as well, I would recommend moving to a SQL Server Express backend. You'll find the move relatively painless and have a faster, more secure database in the long run.

changed the formatting
Source Link
Malachi
  • 28.7k
  • 11
  • 86
  • 188

Is this secure? Well, nothing is ever really secure, but yes. Generally speaking it is. You're using parameterized queries, which is good. Malachi is right though, stored procedures would be even better. What you can do to make this a little more secure is specify the datatype and size of each parameter.

I'm not sure where this code resides, but you should make sure that it is separated from application logic in its own Data Access Layer. You open and close a lot of connections to your back end. That's a performance hit. Implementing a proper DAL should help mitigate that.

What does "3" mean in this piece of code? Nix the magic number and create a constant with a meaningful name.

        For i = 0 To 3
            If thisReader.GetValue(i).ToString() = "" Then
                Throw New SQLException("There has been an error with the database query.")
            End If
        Next

The elephant in the room...

So, now. The elephant in the room.... Access is not a great back end for a website. It has notorious performance issues. Considering you're concerned about security as well, I would recommend moving to a SQL Server Express backend. You'll find the move relatively painless and have a faster, more secure database in the long run.

Is this secure? Well, nothing is ever really secure, but yes. Generally speaking it is. You're using parameterized queries, which is good. Malachi is right though, stored procedures would be even better. What you can do to make this a little more secure is specify the datatype and size of each parameter.

I'm not sure where this code resides, but you should make sure that it is separated from application logic in its own Data Access Layer. You open and close a lot of connections to your back end. That's a performance hit. Implementing a proper DAL should help mitigate that.

What does "3" mean in this piece of code? Nix the magic number and create a constant with a meaningful name.

        For i = 0 To 3
            If thisReader.GetValue(i).ToString() = "" Then
                Throw New SQLException("There has been an error with the database query.")
            End If
        Next

So, now. The elephant in the room.... Access is not a great back end for a website. It has notorious performance issues. Considering you're concerned about security as well, I would recommend moving to a SQL Server Express backend. You'll find the move relatively painless and have a faster, more secure database in the long run.

Is this secure? Well, nothing is ever really secure, but yes. Generally speaking it is. You're using parameterized queries, which is good. Malachi is right though, stored procedures would be even better. What you can do to make this a little more secure is specify the datatype and size of each parameter.

I'm not sure where this code resides, but you should make sure that it is separated from application logic in its own Data Access Layer. You open and close a lot of connections to your back end. That's a performance hit. Implementing a proper DAL should help mitigate that.

What does "3" mean in this piece of code? Nix the magic number and create a constant with a meaningful name.

        For i = 0 To 3
            If thisReader.GetValue(i).ToString() = "" Then
                Throw New SQLException("There has been an error with the database query.")
            End If
        Next

The elephant in the room...

Access is not a great back end for a website. It has notorious performance issues. Considering you're concerned about security as well, I would recommend moving to a SQL Server Express backend. You'll find the move relatively painless and have a faster, more secure database in the long run.

Source Link
RubberDuck
  • 30.9k
  • 6
  • 71
  • 174

Is this secure? Well, nothing is ever really secure, but yes. Generally speaking it is. You're using parameterized queries, which is good. Malachi is right though, stored procedures would be even better. What you can do to make this a little more secure is specify the datatype and size of each parameter.

I'm not sure where this code resides, but you should make sure that it is separated from application logic in its own Data Access Layer. You open and close a lot of connections to your back end. That's a performance hit. Implementing a proper DAL should help mitigate that.

What does "3" mean in this piece of code? Nix the magic number and create a constant with a meaningful name.

        For i = 0 To 3
            If thisReader.GetValue(i).ToString() = "" Then
                Throw New SQLException("There has been an error with the database query.")
            End If
        Next

So, now. The elephant in the room.... Access is not a great back end for a website. It has notorious performance issues. Considering you're concerned about security as well, I would recommend moving to a SQL Server Express backend. You'll find the move relatively painless and have a faster, more secure database in the long run.