HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Checking an MD5 of a device

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
checkingmd5device

Problem

The point of this code snippet is to calculate and compare an MD5 obtained from a USB device connected to the computer against a database to check its validity.

var md5String = string.Empty;
using (var md5 = MD5.Create())
{
    var md5CRC = md5.ComputeHash(File.ReadAllBytes(destinationFilename));
    foreach (byte b in md5CRC)
        md5String += b.ToString("X2");
}

// here I check the MD5 against the database to check the usb device is valid


The code meets its purpose, but I want to know how I can improve this code.

Solution

Using string concatenation in a loop is a no go because each time you do a += a new string object will be created because strings are immutable.

To use the right tool for the job I would suggest to use a StringBuilder instead. Using a StringBuilder doesn't have this restriction because the passed strings are just appended char by char to the internal buffer.

Using braces {} although they might be optional will make your code less error prone.

By extracting the building of the hex string representation to a separate method the responsibility is split nicely and could be reused if needed. Having this as a extension method could serve well.

private string ToHex(this byte[] values)
{
    if (values == null) { throw new ArgumentNullException("values"); }

    StringBuilder sb = new StringBuilder(values.Length * 2);
    foreach (var b in values)
    {
        sb.Append(b.ToString("X2"));
    }

    return sb.ToString();
}


resulting in the former code like this

var md5String = string.Empty;
using (var md5 = MD5.Create())
{
    var md5CRC = md5.ComputeHash(File.ReadAllBytes(destinationFilename));

    md5String = md5CRC.ToHex();
}

Code Snippets

private string ToHex(this byte[] values)
{
    if (values == null) { throw new ArgumentNullException("values"); }

    StringBuilder sb = new StringBuilder(values.Length * 2);
    foreach (var b in values)
    {
        sb.Append(b.ToString("X2"));
    }

    return sb.ToString();
}
var md5String = string.Empty;
using (var md5 = MD5.Create())
{
    var md5CRC = md5.ComputeHash(File.ReadAllBytes(destinationFilename));

    md5String = md5CRC.ToHex();
}

Context

StackExchange Code Review Q#111098, answer score: 5

Revisions (0)

No revisions yet.