Skip to main content
Code Review

Return to Answer

var

You should use them with max_length and salt too. If the type is obvious from the right hand side of the assignment you should use var.

From here here

Under what circumstances is it necessary for a variable's type to be clearly understood when reading the code? Only when the mechanism of the code -- the "how it works" -- is more important to the reader than the semantics -- the "what its for".

So basically in a line like

byte[] salt = new byte[max_length]; 

or

int max_length = 32; 

the type of the variables does not add any value to the code. It is too obvious what the assigned type is, using the type instead of var only adds noise to the code without any real value.

Disposing

If an object implements IDisposable you should either call its Dispose() method or enclose it in an using using block.

Naming

Also variables local to a method aren't mentioned in the naming guidelines naming guidelines I would suggest to use camelCase casing for naming them instead of snake_case casing.

For naming private methods you should use PascalCase casing.

Comments

Comments should only describe why something is done. Let the code itself explain what is done by using meaningful and readable names.

A very good answer about comments can be found here: https://codereview.stackexchange.com/a/90113/29371

getSalt()

You should allow to pass the max_length to the method instead of hardcoding it. This has the advantage that a change to this value won't need the class class/method to be changed. If you make it an optional parameter you can still call it like it whould have no parameter.

Edit

Based on the valid comment from Johnbot you should better use a overloaded GetSalt() method instead of using an optional parameter.

I don't see the need for converting to base64. Encryption algorithms use byte[] arrays, so you should better just return the byte[].


Applying the above would lead to

private static int saltLengthLimit = 32;
private static byte[] GetSalt()
{
 return GetSalt(saltLengthLimit);
}
private static byte[] GetSalt(int maximumSaltLength)
{
 var salt = new byte[maximumSaltLength];
 using (var random = new RNGCryptoServiceProvider())
 {
 random.GetNonZeroBytes(salt);
 }
 return salt;
}

var

You should use them with max_length and salt too. If the type is obvious from the right hand side of the assignment you should use var.

From here

Under what circumstances is it necessary for a variable's type to be clearly understood when reading the code? Only when the mechanism of the code -- the "how it works" -- is more important to the reader than the semantics -- the "what its for".

So basically in a line like

byte[] salt = new byte[max_length]; 

or

int max_length = 32; 

the type of the variables does not add any value to the code. It is too obvious what the assigned type is, using the type instead of var only adds noise to the code without any real value.

Disposing

If an object implements IDisposable you should either call its Dispose() method or enclose it in an using block.

Naming

Also variables local to a method aren't mentioned in the naming guidelines I would suggest to use camelCase casing for naming them instead of snake_case casing.

For naming private methods you should use PascalCase casing.

Comments

Comments should only describe why something is done. Let the code itself explain what is done by using meaningful and readable names.

A very good answer about comments can be found here: https://codereview.stackexchange.com/a/90113/29371

getSalt()

You should allow to pass the max_length to the method instead of hardcoding it. This has the advantage that a change to this value won't need the class class/method to be changed. If you make it an optional parameter you can still call it like it whould have no parameter.

Edit

Based on the valid comment from Johnbot you should better use a overloaded GetSalt() method instead of using an optional parameter.

I don't see the need for converting to base64. Encryption algorithms use byte[] arrays, so you should better just return the byte[].


Applying the above would lead to

private static int saltLengthLimit = 32;
private static byte[] GetSalt()
{
 return GetSalt(saltLengthLimit);
}
private static byte[] GetSalt(int maximumSaltLength)
{
 var salt = new byte[maximumSaltLength];
 using (var random = new RNGCryptoServiceProvider())
 {
 random.GetNonZeroBytes(salt);
 }
 return salt;
}

var

You should use them with max_length and salt too. If the type is obvious from the right hand side of the assignment you should use var.

From here

Under what circumstances is it necessary for a variable's type to be clearly understood when reading the code? Only when the mechanism of the code -- the "how it works" -- is more important to the reader than the semantics -- the "what its for".

So basically in a line like

byte[] salt = new byte[max_length]; 

or

int max_length = 32; 

the type of the variables does not add any value to the code. It is too obvious what the assigned type is, using the type instead of var only adds noise to the code without any real value.

Disposing

If an object implements IDisposable you should either call its Dispose() method or enclose it in an using block.

Naming

Also variables local to a method aren't mentioned in the naming guidelines I would suggest to use camelCase casing for naming them instead of snake_case casing.

For naming private methods you should use PascalCase casing.

Comments

Comments should only describe why something is done. Let the code itself explain what is done by using meaningful and readable names.

A very good answer about comments can be found here: https://codereview.stackexchange.com/a/90113/29371

getSalt()

You should allow to pass the max_length to the method instead of hardcoding it. This has the advantage that a change to this value won't need the class class/method to be changed. If you make it an optional parameter you can still call it like it whould have no parameter.

Edit

Based on the valid comment from Johnbot you should better use a overloaded GetSalt() method instead of using an optional parameter.

I don't see the need for converting to base64. Encryption algorithms use byte[] arrays, so you should better just return the byte[].


Applying the above would lead to

private static int saltLengthLimit = 32;
private static byte[] GetSalt()
{
 return GetSalt(saltLengthLimit);
}
private static byte[] GetSalt(int maximumSaltLength)
{
 var salt = new byte[maximumSaltLength];
 using (var random = new RNGCryptoServiceProvider())
 {
 random.GetNonZeroBytes(salt);
 }
 return salt;
}
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

var

You should use them with max_length and salt too. If the type is obvious from the right hand side of the assignment you should use var.

From here

Under what circumstances is it necessary for a variable's type to be clearly understood when reading the code? Only when the mechanism of the code -- the "how it works" -- is more important to the reader than the semantics -- the "what its for".

So basically in a line like

byte[] salt = new byte[max_length]; 

or

int max_length = 32; 

the type of the variables does not add any value to the code. It is too obvious what the assigned type is, using the type instead of var only adds noise to the code without any real value.

Disposing

If an object implements IDisposable you should either call its Dispose() method or enclose it in an using block.

Naming

Also variables local to a method aren't mentioned in the naming guidelines I would suggest to use camelCase casing for naming them instead of snake_case casing.

For naming private methods you should use PascalCase casing.

Comments

Comments should only describe why something is done. Let the code itself explain what is done by using meaningful and readable names.

A very good answer about comments can be found here: http://codereview.stackexchange.com/a/90113/29371 https://codereview.stackexchange.com/a/90113/29371

getSalt()

You should allow to pass the max_length to the method instead of hardcoding it. This has the advantage that a change to this value won't need the class class/method to be changed. If you make it an optional parameter you can still call it like it whould have no parameter.

Edit

Based on the valid comment comment from Johnbot Johnbot you should better use a overloaded GetSalt() method instead of using an optional parameter.

I don't see the need for converting to base64. Encryption algorithms use byte[] arrays, so you should better just return the byte[].


Applying the above would lead to

private static int saltLengthLimit = 32;
private static byte[] GetSalt()
{
 return GetSalt(saltLengthLimit);
}
private static byte[] GetSalt(int maximumSaltLength)
{
 var salt = new byte[maximumSaltLength];
 using (var random = new RNGCryptoServiceProvider())
 {
 random.GetNonZeroBytes(salt);
 }
 return salt;
}

var

You should use them with max_length and salt too. If the type is obvious from the right hand side of the assignment you should use var.

From here

Under what circumstances is it necessary for a variable's type to be clearly understood when reading the code? Only when the mechanism of the code -- the "how it works" -- is more important to the reader than the semantics -- the "what its for".

So basically in a line like

byte[] salt = new byte[max_length]; 

or

int max_length = 32; 

the type of the variables does not add any value to the code. It is too obvious what the assigned type is, using the type instead of var only adds noise to the code without any real value.

Disposing

If an object implements IDisposable you should either call its Dispose() method or enclose it in an using block.

Naming

Also variables local to a method aren't mentioned in the naming guidelines I would suggest to use camelCase casing for naming them instead of snake_case casing.

For naming private methods you should use PascalCase casing.

Comments

Comments should only describe why something is done. Let the code itself explain what is done by using meaningful and readable names.

A very good answer about comments can be found here: http://codereview.stackexchange.com/a/90113/29371

getSalt()

You should allow to pass the max_length to the method instead of hardcoding it. This has the advantage that a change to this value won't need the class class/method to be changed. If you make it an optional parameter you can still call it like it whould have no parameter.

Edit

Based on the valid comment from Johnbot you should better use a overloaded GetSalt() method instead of using an optional parameter.

I don't see the need for converting to base64. Encryption algorithms use byte[] arrays, so you should better just return the byte[].


Applying the above would lead to

private static int saltLengthLimit = 32;
private static byte[] GetSalt()
{
 return GetSalt(saltLengthLimit);
}
private static byte[] GetSalt(int maximumSaltLength)
{
 var salt = new byte[maximumSaltLength];
 using (var random = new RNGCryptoServiceProvider())
 {
 random.GetNonZeroBytes(salt);
 }
 return salt;
}

var

You should use them with max_length and salt too. If the type is obvious from the right hand side of the assignment you should use var.

From here

Under what circumstances is it necessary for a variable's type to be clearly understood when reading the code? Only when the mechanism of the code -- the "how it works" -- is more important to the reader than the semantics -- the "what its for".

So basically in a line like

byte[] salt = new byte[max_length]; 

or

int max_length = 32; 

the type of the variables does not add any value to the code. It is too obvious what the assigned type is, using the type instead of var only adds noise to the code without any real value.

Disposing

If an object implements IDisposable you should either call its Dispose() method or enclose it in an using block.

Naming

Also variables local to a method aren't mentioned in the naming guidelines I would suggest to use camelCase casing for naming them instead of snake_case casing.

For naming private methods you should use PascalCase casing.

Comments

Comments should only describe why something is done. Let the code itself explain what is done by using meaningful and readable names.

A very good answer about comments can be found here: https://codereview.stackexchange.com/a/90113/29371

getSalt()

You should allow to pass the max_length to the method instead of hardcoding it. This has the advantage that a change to this value won't need the class class/method to be changed. If you make it an optional parameter you can still call it like it whould have no parameter.

Edit

Based on the valid comment from Johnbot you should better use a overloaded GetSalt() method instead of using an optional parameter.

I don't see the need for converting to base64. Encryption algorithms use byte[] arrays, so you should better just return the byte[].


Applying the above would lead to

private static int saltLengthLimit = 32;
private static byte[] GetSalt()
{
 return GetSalt(saltLengthLimit);
}
private static byte[] GetSalt(int maximumSaltLength)
{
 var salt = new byte[maximumSaltLength];
 using (var random = new RNGCryptoServiceProvider())
 {
 random.GetNonZeroBytes(salt);
 }
 return salt;
}
added 484 characters in body
Source Link
Heslacher
  • 50.9k
  • 5
  • 83
  • 177

var

You should use them with max_length and salt too. If the type is obvious from the right hand side of the assignment you should use var.

From here

Under what circumstances is it necessary for a variable's type to be clearly understood when reading the code? Only when the mechanism of the code -- the "how it works" -- is more important to the reader than the semantics -- the "what its for".

So basically in a line like

byte[] salt = new byte[max_length]; 

or

int max_length = 32; 

the type of the variables does not add any value to the code. It is too obvious what the assigned type is, using the type instead of var only adds noise to the code without any real value.

Disposing

If an object implements IDisposable you should either call its Dispose() method or enclose it in an using block.

Naming

Also variables local to a method aren't mentioned in the naming guidelines I would suggest to use camelCase casing for naming them instead of snake_case casing.

For naming private methods you should use PascalCase casing.

Comments

Comments should only describe why something is done. Let the code itself explain what is done by using meaningful and readable names.

A very good answer about comments can be found here: http://codereview.stackexchange.com/a/90113/29371

getSalt()

You should allow to pass the max_length to the method instead of hardcoding it. This has the advantage that a change to this value won't need the class class/method to be changed. If you make it an optional parameter you can still call it like it whould have no parameter.

Edit

Based on the valid comment from Johnbot you should better use a overloaded GetSalt() method instead of using an optional parameter.

I don't see the need for converting to base64. Encryption algorithms use byte[] arrays, so you should better just return the byte[].


Applying the above would lead to

private static int saltLengthLimit = 32;
private static byte[] GetSalt(int)
{
 maximumSaltLength = 32 return GetSalt(saltLengthLimit);
}
private static byte[] GetSalt(int maximumSaltLength)
{
 var salt = new byte[maximumSaltLength];
 using (var random = new RNGCryptoServiceProvider())
 {
 random.GetNonZeroBytes(salt);
 }
 return salt;
}

var

You should use them with max_length and salt too. If the type is obvious from the right hand side of the assignment you should use var.

From here

Under what circumstances is it necessary for a variable's type to be clearly understood when reading the code? Only when the mechanism of the code -- the "how it works" -- is more important to the reader than the semantics -- the "what its for".

So basically in a line like

byte[] salt = new byte[max_length]; 

or

int max_length = 32; 

the type of the variables does not add any value to the code. It is too obvious what the assigned type is, using the type instead of var only adds noise to the code without any real value.

Disposing

If an object implements IDisposable you should either call its Dispose() method or enclose it in an using block.

Naming

Also variables local to a method aren't mentioned in the naming guidelines I would suggest to use camelCase casing for naming them instead of snake_case casing.

For naming private methods you should use PascalCase casing.

Comments

Comments should only describe why something is done. Let the code itself explain what is done by using meaningful and readable names.

A very good answer about comments can be found here: http://codereview.stackexchange.com/a/90113/29371

getSalt()

You should allow to pass the max_length to the method instead of hardcoding it. This has the advantage that a change to this value won't need the class class/method to be changed. If you make it an optional parameter you can still call it like it whould have no parameter.

I don't see the need for converting to base64. Encryption algorithms use byte[] arrays, so you should better just return the byte[].


Applying the above would lead to

private static byte[] GetSalt(int maximumSaltLength = 32)
{
 var salt = new byte[maximumSaltLength];
 using (var random = new RNGCryptoServiceProvider())
 {
 random.GetNonZeroBytes(salt);
 }
 return salt;
}

var

You should use them with max_length and salt too. If the type is obvious from the right hand side of the assignment you should use var.

From here

Under what circumstances is it necessary for a variable's type to be clearly understood when reading the code? Only when the mechanism of the code -- the "how it works" -- is more important to the reader than the semantics -- the "what its for".

So basically in a line like

byte[] salt = new byte[max_length]; 

or

int max_length = 32; 

the type of the variables does not add any value to the code. It is too obvious what the assigned type is, using the type instead of var only adds noise to the code without any real value.

Disposing

If an object implements IDisposable you should either call its Dispose() method or enclose it in an using block.

Naming

Also variables local to a method aren't mentioned in the naming guidelines I would suggest to use camelCase casing for naming them instead of snake_case casing.

For naming private methods you should use PascalCase casing.

Comments

Comments should only describe why something is done. Let the code itself explain what is done by using meaningful and readable names.

A very good answer about comments can be found here: http://codereview.stackexchange.com/a/90113/29371

getSalt()

You should allow to pass the max_length to the method instead of hardcoding it. This has the advantage that a change to this value won't need the class class/method to be changed. If you make it an optional parameter you can still call it like it whould have no parameter.

Edit

Based on the valid comment from Johnbot you should better use a overloaded GetSalt() method instead of using an optional parameter.

I don't see the need for converting to base64. Encryption algorithms use byte[] arrays, so you should better just return the byte[].


Applying the above would lead to

private static int saltLengthLimit = 32;
private static byte[] GetSalt()
{
  return GetSalt(saltLengthLimit);
}
private static byte[] GetSalt(int maximumSaltLength)
{
 var salt = new byte[maximumSaltLength];
 using (var random = new RNGCryptoServiceProvider())
 {
 random.GetNonZeroBytes(salt);
 }
 return salt;
}
added 691 characters in body
Source Link
Heslacher
  • 50.9k
  • 5
  • 83
  • 177
Loading
Source Link
Heslacher
  • 50.9k
  • 5
  • 83
  • 177
Loading
lang-cs

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