Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

Let us focusing first on the code in question.

Naming

Based on the naming guidelines methods should be named using PascalCase casing and method parameters should be named using camelCase casing.

If you have your own coding/naming guidelines you should stick to them. Right now I don't believe so because you are mixing the casing styles for the input parameter names.

Based on the same guidelines one shouldn't use abbreviations for names. A variable named CS doesn't make it obvious at first glance what it is about, but connectionString would do.


Catching Exception isn't really a good way to do error handling. You should catch only the type of exception which can be handled by your code. What I mean with this can be read here here

  • Always deal with known exceptions as low-down as you can. However, if you're expecting an exception it's usually better practice to test for it first. For instance parse, formatting and arithmetic exceptions are nearly always better handled by logic checks first, rather than a specific try-catch.

  • If you need to do something on an exception (for instance logging or roll back a transaction) then re-throw the exception.

  • Always deal with unknown exceptions as high-up as you can - the only code that should consume an exception and not re-throw it should be the UI or public API.


If you want to make this reusable for other update methods, you should pass a Dictionary<TKey,TValue> to the method, where as TKey will be the parameters "name" like @LastName and the TValue will be the values the parameter will/should have like so

Public Sub UpdateRecord(ByRef procedure As String, Dictionary<String, Object> parameters)

You can then add the keys as the parameters name and the values as the value.

Let us focusing first on the code in question.

Naming

Based on the naming guidelines methods should be named using PascalCase casing and method parameters should be named using camelCase casing.

If you have your own coding/naming guidelines you should stick to them. Right now I don't believe so because you are mixing the casing styles for the input parameter names.

Based on the same guidelines one shouldn't use abbreviations for names. A variable named CS doesn't make it obvious at first glance what it is about, but connectionString would do.


Catching Exception isn't really a good way to do error handling. You should catch only the type of exception which can be handled by your code. What I mean with this can be read here

  • Always deal with known exceptions as low-down as you can. However, if you're expecting an exception it's usually better practice to test for it first. For instance parse, formatting and arithmetic exceptions are nearly always better handled by logic checks first, rather than a specific try-catch.

  • If you need to do something on an exception (for instance logging or roll back a transaction) then re-throw the exception.

  • Always deal with unknown exceptions as high-up as you can - the only code that should consume an exception and not re-throw it should be the UI or public API.


If you want to make this reusable for other update methods, you should pass a Dictionary<TKey,TValue> to the method, where as TKey will be the parameters "name" like @LastName and the TValue will be the values the parameter will/should have like so

Public Sub UpdateRecord(ByRef procedure As String, Dictionary<String, Object> parameters)

You can then add the keys as the parameters name and the values as the value.

Let us focusing first on the code in question.

Naming

Based on the naming guidelines methods should be named using PascalCase casing and method parameters should be named using camelCase casing.

If you have your own coding/naming guidelines you should stick to them. Right now I don't believe so because you are mixing the casing styles for the input parameter names.

Based on the same guidelines one shouldn't use abbreviations for names. A variable named CS doesn't make it obvious at first glance what it is about, but connectionString would do.


Catching Exception isn't really a good way to do error handling. You should catch only the type of exception which can be handled by your code. What I mean with this can be read here

  • Always deal with known exceptions as low-down as you can. However, if you're expecting an exception it's usually better practice to test for it first. For instance parse, formatting and arithmetic exceptions are nearly always better handled by logic checks first, rather than a specific try-catch.

  • If you need to do something on an exception (for instance logging or roll back a transaction) then re-throw the exception.

  • Always deal with unknown exceptions as high-up as you can - the only code that should consume an exception and not re-throw it should be the UI or public API.


If you want to make this reusable for other update methods, you should pass a Dictionary<TKey,TValue> to the method, where as TKey will be the parameters "name" like @LastName and the TValue will be the values the parameter will/should have like so

Public Sub UpdateRecord(ByRef procedure As String, Dictionary<String, Object> parameters)

You can then add the keys as the parameters name and the values as the value.

fixed the formatting
Source Link
Malachi
  • 29k
  • 11
  • 86
  • 188

Let us focusing first on the code in question.

Naming

Based on the naming guidelines methods should be named using PascalCase casing and method parameters should be named using camelCase casing.

If you have your own coding/naming guidelines you should stick to them. Right now I don't believe so because you are mixing the casing styles for the input parameternamesparameter names.

Based on the same guidelines one shouldn't use abbreviations for names. A variable named CS doesn't make it obvious at first glance what it is about, but connectionString would do.


Catching Exception isn't really a good way to do error handling. You should catch only the type of exception which can be handled by your code. What I mean with this can be read here

  • Always deal with known exceptions as low-down as you can. However, if you're expecting an exception it's usually better practice to test for it first. For instance parse, formatting and arithmetic exceptions are nearly always better handled by logic checks first, rather than a specific try-catch.

  • If you need to do something on an exception (for instance logging or roll back a transaction) then re-throw the exception.

  • Always deal with unknown exceptions as high-up as you can - the only code that should consume an exception and not re-throw it should be the UI or public API.


If you want to make this reusable for other update methods, you should pass a Dictionary<TKey,TValue> to the method, where as TKey will be the parameters "name" like @LastName" and the @LastNameTValue` and the TValue will be the values the parameter will/should have like so

Public Sub UpdateRecord(ByRef procedure As String, Dictionary<String, Object> parameters)

You can then add the keys as the parameters name and the values as the value.

Let us focusing first on the code in question.

Naming

Based on the naming guidelines methods should be named using PascalCase casing and method parameters should be named using camelCase casing.

If you have your own coding/naming guidelines you should stick to them. Right now I don't believe so because you are mixing the casing styles for the input parameternames.

Based on the same guidelines one shouldn't use abbreviations for names. A variable named CS doesn't make it obvious at first glance what it is about, but connectionString would do.


Catching Exception isn't really a good way to do error handling. You should catch only the type of exception which can be handled by your code. What I mean with this can be read here

  • Always deal with known exceptions as low-down as you can. However, if you're expecting an exception it's usually better practice to test for it first. For instance parse, formatting and arithmetic exceptions are nearly always better handled by logic checks first, rather than a specific try-catch.

  • If you need to do something on an exception (for instance logging or roll back a transaction) then re-throw the exception.

  • Always deal with unknown exceptions as high-up as you can - the only code that should consume an exception and not re-throw it should be the UI or public API.


If you want to make this reusable for other update methods, you should pass a Dictionary<TKey,TValue> to the method, where as TKey will be the parameters "name" like @LastName" and the TValue` will be the values the parameter will/should have like so

Public Sub UpdateRecord(ByRef procedure As String, Dictionary<String, Object> parameters)

You can then add the keys as the parameters name and the values as the value.

Let us focusing first on the code in question.

Naming

Based on the naming guidelines methods should be named using PascalCase casing and method parameters should be named using camelCase casing.

If you have your own coding/naming guidelines you should stick to them. Right now I don't believe so because you are mixing the casing styles for the input parameter names.

Based on the same guidelines one shouldn't use abbreviations for names. A variable named CS doesn't make it obvious at first glance what it is about, but connectionString would do.


Catching Exception isn't really a good way to do error handling. You should catch only the type of exception which can be handled by your code. What I mean with this can be read here

  • Always deal with known exceptions as low-down as you can. However, if you're expecting an exception it's usually better practice to test for it first. For instance parse, formatting and arithmetic exceptions are nearly always better handled by logic checks first, rather than a specific try-catch.

  • If you need to do something on an exception (for instance logging or roll back a transaction) then re-throw the exception.

  • Always deal with unknown exceptions as high-up as you can - the only code that should consume an exception and not re-throw it should be the UI or public API.


If you want to make this reusable for other update methods, you should pass a Dictionary<TKey,TValue> to the method, where as TKey will be the parameters "name" like @LastName and the TValue will be the values the parameter will/should have like so

Public Sub UpdateRecord(ByRef procedure As String, Dictionary<String, Object> parameters)

You can then add the keys as the parameters name and the values as the value.

Source Link
Heslacher
  • 50.9k
  • 5
  • 83
  • 177

Let us focusing first on the code in question.

Naming

Based on the naming guidelines methods should be named using PascalCase casing and method parameters should be named using camelCase casing.

If you have your own coding/naming guidelines you should stick to them. Right now I don't believe so because you are mixing the casing styles for the input parameternames.

Based on the same guidelines one shouldn't use abbreviations for names. A variable named CS doesn't make it obvious at first glance what it is about, but connectionString would do.


Catching Exception isn't really a good way to do error handling. You should catch only the type of exception which can be handled by your code. What I mean with this can be read here

  • Always deal with known exceptions as low-down as you can. However, if you're expecting an exception it's usually better practice to test for it first. For instance parse, formatting and arithmetic exceptions are nearly always better handled by logic checks first, rather than a specific try-catch.

  • If you need to do something on an exception (for instance logging or roll back a transaction) then re-throw the exception.

  • Always deal with unknown exceptions as high-up as you can - the only code that should consume an exception and not re-throw it should be the UI or public API.


If you want to make this reusable for other update methods, you should pass a Dictionary<TKey,TValue> to the method, where as TKey will be the parameters "name" like @LastName" and the TValue` will be the values the parameter will/should have like so

Public Sub UpdateRecord(ByRef procedure As String, Dictionary<String, Object> parameters)

You can then add the keys as the parameters name and the values as the value.

default

AltStyle によって変換されたページ (->オリジナル) /