2
\$\begingroup\$

Is there a better way of doing this? I heard that BinaryReader and Exif parsing to get the property item is faster, but I have no idea how to do that, thanks for the help.

//we init this once so that if the function is repeatedly called
//it isn't stressing the garbage man
static Regex r = new Regex(":");

//retrieves the datetime WITHOUT loading the whole image
string[] GetDateTakenFromImage(string path)
{
    try
    {
        using (FileStream fs = new FileStream(path, FileMode.Open, FileAccess.Read))
        {
            using (Image myImage = Image.FromStream(fs, false, false))
            {
                PropertyItem propItem = myImage.GetPropertyItem(36867);
                string dateTaken = r.Replace(Encoding.UTF8.GetString(propItem.Value), "-", 2);
                if(string.IsNullOrEmpty(dateTaken))
                    return null;
                else
                    return DateTime.Parse(dateTaken).ToString("yyyy-MM-dd").Split('-');
            }
        }
    }
    catch
    {
        return null;
    }
}
\$\endgroup\$
2
\$\begingroup\$
//retrieves the datetime WITHOUT loading the whole image
string[] GetDateTakenFromImage(string path)

Function name and parameter are named well. You should use an XML-doc header to document the quirk that it doesn't need to load the entire image.

Overall I like that the method is concise: it mostly does only one thing, it's easy to read, and not needing to pre-load the whole image is a nice bonus.

It is strange to use string[] to denote a date. You should be returning a DateTime?.

Consider changing it to accept a Stream instead of a string path. Currently it's a bit burdensome to test your method because it requires a file path, even though all it's doing is getting a stream out of it anyway. By accepting a Stream instead, you can more easily put automated tests around it that use in-memory test data and avoid a whole litany of IO nonsense.


using (FileStream fs = new FileStream(path, FileMode.Open, FileAccess.Read))

fs is a poor name. Give it some more meaning, like imageStream. It can also be written a bit more concisely:

using (FileStream imageStream = File.OpenRead(path))

Likewise, myImage could just be named image.


PropertyItem propItem = myImage.GetPropertyItem(36867);

Avoid magic numbers -- that 36867 should be in a named constant somewhere:

const int ExifDateTimeOriginal = 0x9003;

Your error handling in general could be improved. If I was consuming this API, I would naturally expect exceptions relating to IO (file not found, file not accessible, not a valid image file, and so on) to propagate up. It's up to you whether you want to throw or return null in the case where everything is valid but the image simply doesn't have that tag.

You're returning null if anything goes wrong which makes this harder to test. Be aware that myImage.GetPropertyItem(36867); will throw if the tag is not present (which in my opinion is a totally non-exceptional circumstance), so if you do tweak your method to propagate other exceptions you will need to put that one line around a try-catch for that one exception.


The EXIF tag you're checking is encoded in ASCII according to the EXIF docs I've been able to find, so this should use Encoding.ASCII instead of Encoding.UTF8:

string dateTaken = r.Replace(Encoding.UTF8.GetString(propItem.Value), "-", 2);

You also don't need to do any string replacing. DateTime.ParseExact is handy for parsing dates encoded in custom formats:

string dateTaken = Encoding.ASCII.GetString(propItem.Value);
...
return DateTime.ParseExact(dateTaken.Trim('\0'), "yyyy:MM:dd HH:mm:ss", CultureInfo.InvariantCulture);

Lastly, if you want to really adhere to the letter of the spec then depending on how you decide to modify or keep your method contract you'll need to handle the case where the date and time are unknown and all non-colon characters are replaced with spaces.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.