Style
private const int _saltSizeBytes = 32;
private const int _IVSizeBytes = 16;
private const int _PBKDF2Iterations = 10000;
Initially, I wanted to comment that consts should be written in all caps, e.g. SALT_SIZE_BYTES
. However, this answer disagrees based on Microsoft's StyleCop rules.
The recommended naming and capitalization convention is to use Pascal casing for constants (Microsoft has a tool named StyleCop that documents all the preferred conventions and can check your source for compliance - though it is a little bit too anally retentive for many people's tastes). e.g.
private const int TheAnswer = 42;
I'm still used to the all caps, but pascal casing seems to be a better option based on what I find online.
The style you're currently using is used for private fields; which is not what you want to use here.
Comments
The method summaries are great to have. Even if a bit obvious at times, it's still good to have them in regards to Intellisense tooltips.
You put a lot of effort in writing comments, but you went a bit overboard with it. Obvious comments that rephrase a line of code should be avoided unless they meaningfully add an explanation. Some examples below:
//Set ciphermode for the AES algoritm (CBC, cipher block chaining, by default)
aesManaged.Mode = cipherMode;
There's no need to list the options. That's what the enum is for. Now you have two places to updates if the enum changes: the enum and this comment. That's more work than you want.
Furthermore, Set ciphermode for the AES algoritm
is obvious when you look at the code: aesManaged.Mode = cipherMode
.
The comment doesn't add much explanation, so it can be removed.
//Generate a random salt
byte[] salt = GenerateRandomSalt();
The comment is already explained by the method name.
//Open filestream
using (FileStream outputFileStream = new FileStream("../../Files/" + fileNameAndExtension, FileMode.Create))
The comment doesn't add something that wasn't already clear.
//Append salt to filestream
outputFileStream.Write(salt, 0, salt.Length);
The comment is just a rephrase of outputFileStream.Write(salt
.
//Initialize byte array to store salt, the salt is 32 bytes (256 bits) long
byte[] salt = new byte[32];
The comment lists what the code tells us. From the code, I can see that you're making a byte array that represents the salt and is 32 bytes in size.
As a counterexample, let's say that you commonly refer to the salt as "a 256 bit salt" in the entire application. Adding a comment that reflects that can be meaningful, as the relation between 32 and 256 is not immediately apparent:
byte[] salt = new byte[32]; //256 bits
This comment adds something that was not already obvious.
I'm not all that experience in cryptography so I can't comment on the intention of the code.
Style
private const int _saltSizeBytes = 32;
private const int _IVSizeBytes = 16;
private const int _PBKDF2Iterations = 10000;
Initially, I wanted to comment that consts should be written in all caps, e.g. SALT_SIZE_BYTES
. However, this answer disagrees based on Microsoft's StyleCop rules.
The recommended naming and capitalization convention is to use Pascal casing for constants (Microsoft has a tool named StyleCop that documents all the preferred conventions and can check your source for compliance - though it is a little bit too anally retentive for many people's tastes). e.g.
private const int TheAnswer = 42;
I'm still used to the all caps, but pascal casing seems to be a better option based on what I find online.
The style you're currently using is used for private fields; which is not what you want to use here.
Comments
The method summaries are great to have. Even if a bit obvious at times, it's still good to have them in regards to Intellisense tooltips.
You put a lot of effort in writing comments, but you went a bit overboard with it. Obvious comments that rephrase a line of code should be avoided unless they meaningfully add an explanation. Some examples below:
//Set ciphermode for the AES algoritm (CBC, cipher block chaining, by default)
aesManaged.Mode = cipherMode;
There's no need to list the options. That's what the enum is for. Now you have two places to updates if the enum changes: the enum and this comment. That's more work than you want.
Furthermore, Set ciphermode for the AES algoritm
is obvious when you look at the code: aesManaged.Mode = cipherMode
.
The comment doesn't add much explanation, so it can be removed.
//Generate a random salt
byte[] salt = GenerateRandomSalt();
The comment is already explained by the method name.
//Open filestream
using (FileStream outputFileStream = new FileStream("../../Files/" + fileNameAndExtension, FileMode.Create))
The comment doesn't add something that wasn't already clear.
//Append salt to filestream
outputFileStream.Write(salt, 0, salt.Length);
The comment is just a rephrase of outputFileStream.Write(salt
.
//Initialize byte array to store salt, the salt is 32 bytes (256 bits) long
byte[] salt = new byte[32];
The comment lists what the code tells us. From the code, I can see that you're making a byte array that represents the salt and is 32 bytes in size.
As a counterexample, let's say that you commonly refer to the salt as "a 256 bit salt" in the entire application. Adding a comment that reflects that can be meaningful, as the relation between 32 and 256 is not immediately apparent:
byte[] salt = new byte[32]; //256 bits
This comment adds something that was not already obvious.
I'm not all that experience in cryptography so I can't comment on the intention of the code.
Style
private const int _saltSizeBytes = 32;
private const int _IVSizeBytes = 16;
private const int _PBKDF2Iterations = 10000;
Initially, I wanted to comment that consts should be written in all caps, e.g. SALT_SIZE_BYTES
. However, this answer disagrees based on Microsoft's StyleCop rules.
The recommended naming and capitalization convention is to use Pascal casing for constants (Microsoft has a tool named StyleCop that documents all the preferred conventions and can check your source for compliance - though it is a little bit too anally retentive for many people's tastes). e.g.
private const int TheAnswer = 42;
I'm still used to the all caps, but pascal casing seems to be a better option based on what I find online.
The style you're currently using is used for private fields; which is not what you want to use here.
Comments
The method summaries are great to have. Even if a bit obvious at times, it's still good to have them in regards to Intellisense tooltips.
You put a lot of effort in writing comments, but you went a bit overboard with it. Obvious comments that rephrase a line of code should be avoided unless they meaningfully add an explanation. Some examples below:
//Set ciphermode for the AES algoritm (CBC, cipher block chaining, by default)
aesManaged.Mode = cipherMode;
There's no need to list the options. That's what the enum is for. Now you have two places to updates if the enum changes: the enum and this comment. That's more work than you want.
Furthermore, Set ciphermode for the AES algoritm
is obvious when you look at the code: aesManaged.Mode = cipherMode
.
The comment doesn't add much explanation, so it can be removed.
//Generate a random salt
byte[] salt = GenerateRandomSalt();
The comment is already explained by the method name.
//Open filestream
using (FileStream outputFileStream = new FileStream("../../Files/" + fileNameAndExtension, FileMode.Create))
The comment doesn't add something that wasn't already clear.
//Append salt to filestream
outputFileStream.Write(salt, 0, salt.Length);
The comment is just a rephrase of outputFileStream.Write(salt
.
//Initialize byte array to store salt, the salt is 32 bytes (256 bits) long
byte[] salt = new byte[32];
The comment lists what the code tells us. From the code, I can see that you're making a byte array that represents the salt and is 32 bytes in size.
As a counterexample, let's say that you commonly refer to the salt as "a 256 bit salt" in the entire application. Adding a comment that reflects that can be meaningful, as the relation between 32 and 256 is not immediately apparent:
byte[] salt = new byte[32]; //256 bits
This comment adds something that was not already obvious.
I'm not all that experience in cryptography so I can't comment on the intention of the code.
Style
private const int _saltSizeBytes = 32;
private const int _IVSizeBytes = 16;
private const int _PBKDF2Iterations = 10000;
Initially, I wanted to comment that consts should be written in all caps, e.g. SALT_SIZE_BYTES
. However, this answer disagrees based on Microsoft's StyleCop rules.
The recommended naming and capitalization convention is to use Pascal casing for constants (Microsoft has a tool named StyleCop that documents all the preferred conventions and can check your source for compliance - though it is a little bit too anally retentive for many people's tastes). e.g.
private const int TheAnswer = 42;
I'm still used to the all caps, but pascal casing seems to be a better option based on what I find online.
The style you're currently using is used for private fields; which is not what you want to use here.
Comments
The method summaries are great to have. Even if a bit obvious at times, it's still good to have them in regards to Intellisense tooltips.
You put a lot of effort in writing comments, but you went a bit overboard with it. Obvious comments that rephrase a line of code should be avoided unless they meaningfully add an explanation. Some examples below:
//Set ciphermode for the AES algoritm (CBC, cipher block chaining, by default)
aesManaged.Mode = cipherMode;
There's no need to list the options. That's what the enum is for. Now you have two places to updates if the enum changes: the enum and this comment. That's more work than you want.
Furthermore, Set ciphermode for the AES algoritm
is obvious when you look at the code: aesManaged.Mode = cipherMode
.
The comment doesn't add much explanation, so it can be removed.
//Generate a random salt
byte[] salt = GenerateRandomSalt();
The comment is already explained by the method name.
//Open filestream
using (FileStream outputFileStream = new FileStream("../../Files/" + fileNameAndExtension, FileMode.Create))
The comment doesn't add something that wasn't already clear.
//Append salt to filestream
outputFileStream.Write(salt, 0, salt.Length);
The comment is just a rephrase of outputFileStream.Write(salt
.
//Initialize byte array to store salt, the salt is 32 bytes (256 bits) long
byte[] salt = new byte[32];
The comment lists what the code tells us. From the code, I can see that you're making a byte array that represents the salt and is 32 bytes in size.
As a counterexample, let's say that you commonly refer to the salt as "a 256 bit salt" in the entire application. Adding a comment that reflects that can be meaningful, as the relation between 32 and 256 is not immediately apparent:
byte[] salt = new byte[32]; //256 bits
This comment adds something that was not already obvious.
I'm not all that experience in cryptography so I can't comment on the intention of the code.