Hi all,

I have way too many pictures on my PC and they are everywhere. I am sure many of you have same problem.

I am attempting to create a simple program to organize them a bit and minimizing manual work, but unfortunately there will still be some.

Please take a look through this code and let me know if you have any suggestions. I am not very happy with how different pictures with same name is handled. How can I improve this code?

Thanks!

//Start Execution here...
private void startBtn_Click(object sender, EventArgs e)
{
	DirectoryInfo di = new DirectoryInfo("C:\\srcPictures");
	WalkDirectoryTree(di);
}

private void WalkDirectoryTree(System.IO.DirectoryInfo root)
{
	FileInfo[] files = null;
	DirectoryInfo[] subDirs = null;

	// First process files under root folder
	files = root.GetFiles("*.*");
	
	Logger("Processing Files under: " + root.FullName);

	if (files != null)
	{
		foreach (FileInfo fi in files)
		{
			string picTakenOn = getDateTaken(fi, false);
			string targetDir = "";

			if (picTakenOn.Contains("DATETAKEN"))
			{
				picTakenOn = picTakenOn.Replace("DATETAKEN", "");
				targetDir = "C:\\organizedPictures\\" + picTakenOn + "\\";
				if (!Directory.Exists(targetDir))
				{
					Directory.CreateDirectory(targetDir);
				}
				copyFileToDestination(fi, fi.FullName, targetDir + fi.Name);
			}
			else if (picTakenOn.Contains("DATEMODIFIED"))
			{
				picTakenOn = picTakenOn.Replace("DATEMODIFIED", "");
				targetDir = destDirectory.Text + "\\Sorted_DateModified\\" + picTakenOn + "\\";
				if (!Directory.Exists(targetDir))
				{
					Directory.CreateDirectory(targetDir);
				}
				Logger("DATEMODIFIED used instead of DATETAKEN for: " + fi.FullName);
				copyFileToDestination(fi, fi.FullName, targetDir + fi.Name);
			}
			else if (picTakenOn.Contains("INVALID"))
			{
				Logger("Not an image (Skipped): " + fi.FullName);
			}
		}

		// Resursion for all subdirectories
		subDirs = root.GetDirectories();
		foreach (System.IO.DirectoryInfo dirInfo in subDirs)
		{
			WalkDirectoryTree(dirInfo);
		}
	}
}

private void copyFileToDestination(FileInfo srcInfo, string sourcePath, string destPath)
{
	if (File.Exists(destPath)) //Same file name already exists
	{
		FileInfo destInfo = new FileInfo(destPath);
		if (srcInfo.Length != destInfo.Length) //If different images (check by filesize)
		{
			Logger("FILENAME conflict for: " + sourcePath);
			copyFileToDestination(srcInfo, sourcePath, destPath + "_dup" + srcInfo.Extension);
		}
		else
		{
			Logger("SKIPPED copy (Dup found) for: " + sourcePath);
		}
	}
	else
	{
		File.Copy(sourcePath, destPath);
	}
}

private string getDateTaken(FileInfo fi, bool includeDate)
{
	if (fi.Extension != ".jpeg" && fi.Extension != ".jpg" && fi.Extension != ".JPEG" && fi.Extension != ".JPG")
		return "INVALID";

	Regex r = new Regex(":");

	FileStream fs = new FileStream(fi.FullName, FileMode.Open, FileAccess.Read);
	Image myImage = Image.FromStream(fs, false, false);

	try
	{
		PropertyItem propItem = myImage.GetPropertyItem(36867); //36867 is for Date taken
		string dateTaken = r.Replace(Encoding.UTF8.GetString(propItem.Value), "-", 2).Split(' ')[0];
		return ("DATETAKEN" + dateTaken.Split('-')[0] + "-" + dateTaken.Split('-')[1]);
	}
	catch
	{
		string dateModified = fi.LastWriteTime.ToString();
		return ("DATEMODIFIED" + dateModified.Split(' ')[0].Split('/')[2] + "-" + dateModified.Split(' ')[0].Split('/')[1]);
	}
}

public void Logger(String lines)
{
	//Set the log file
	string logFile = "C:\\organizedPictures\\log.txt";

	System.IO.StreamWriter file = new System.IO.StreamWriter(logFile, true);
	file.WriteLine(lines);

	file.Close();
}

Ok, so a couple of comments...

you have some code duplication here that could be trivially refactored out...

				if (!Directory.Exists(targetDir))
				{
					Directory.CreateDirectory(targetDir);
				}
				copyFileToDestination(fi, fi.FullName, targetDir + fi.Name);

could be moved to after the if statement, then simply add a continue; to the final else if condition.

Your duplicate image checking isn't... very good. It would be better off to hash the two images and compare the hashes to see if they are the same. Many images can have the same length, same name, and be completely different (especially if you start dealing in raws).

You're returning a combined string in your getDateTaken method. Either use output parameters or return a Tuple<string, string> instead. Or better yet, a Tuple<DateTakenType, DateTime> where DateTakenType is an enumeration of Invalid, Taken, Modified. This would allow you to completely eliminate the majority of the previous if statements (including the nasty string operations) and simply change it to a switch based on the enumeration type.

You use a destDirectory in one if statement, and not in the other. That appears to be a bug. Hard coding of paths is a bad idea, I highly recommend not doing that. Things like your log should probably be stored in the destination directory. The source directory should also be dynamic.

GetFiles will filter based on extension, it is not case sensitive.

Use Directory.EnumerateFiles to enumerate all files in a directory and subdirectory.

Use StringComparison.OrdinalIgnoreCase

Taking all of that into consideration... you might start refactoring it into something closer to:

private void startBtn_Click(object sender, EventArgs e)
        {
            var srcImages = Directory.EnumerateFiles(srcDirectory, "*.*", SearchOption.AllDirectories).Where(s => s.EndsWith(".jpg", StringComparison.OrdinalIgnoreCase) || s.EndsWith(".jpeg", StringComparison.OrdinalIgnoreCase));
            MoveImagesAroundPlease(srcImages, destDirectory);
        }

        private void MoveImagesAroundPlease(IEnumerable<string> srcImages, string destDirectory)
        {
            foreach (var srcImagePath in srcImages)
            {
                FileInfo srcImage = new FileInfo(srcImagePath);
                var destPath = destDirectory;

                var result = GetDateTaken(srcImage);

                switch (result.Item1)
                {
                    case ImageDateTakenType.Created:
                        destPath += result.Item2.ToString("yyyy-MM") + "\\";
                        break;
                    case ImageDateTakenType.Modified:
                        destPath += "\\Sorted_DateModified\\" + result.Item2.ToString("yyyy-MM") + "\\";
                        Log("Date modified used instead of date taken for: " + srcImage.FullName);
                        break;
                }

                if (!Directory.Exists(destPath))
                {
                    Directory.CreateDirectory(destPath);
                }

                CopyFileToDestination(srcImage, destPath + srcImage.Name);
            }
        }

        private Tuple<ImageDateTakenType, DateTime> GetDateTaken(FileInfo srcImageFile)
        {
            FileStream fs = new FileStream(srcImageFile.FullName, FileMode.Open, FileAccess.Read);
            Image srcImage = Image.FromStream(fs, false, false);

            try
            {
                PropertyItem dateTakenProperty = srcImage.GetPropertyItem(36867); //36867 is for Date taken
                return new Tuple<ImageDateTakenType, DateTime>(ImageDateTakenType.Created, DateTime.Parse(Encoding.UTF8.GetString(dateTakenProperty.Value)));
            }
            catch
            {
                return new Tuple<ImageDateTakenType, DateTime>(ImageDateTakenType.Modified, srcImageFile.LastWriteTime);
            }
        }

        private void CopyFileToDestination(FileInfo srcImageFile, string destPath)
        {
            if (File.Exists(destPath)) //Same file name already exists
            {
                FileInfo destInfo = new FileInfo(destPath);
                int i = 0;
                while (true)
                {
                    if (srcImageFile.Length != destInfo.Length) //If different images (check by filesize)
                    {
                        Log("FILENAME conflict for: " + srcImageFile);
                        CopyFileToDestination(srcImageFile, destInfo.DirectoryName + "\\" + Path.GetFileNameWithoutExtension(destInfo.Name) + "_dup" + i + srcImageFile.Extension);
                        break;
                    }
                    else
                    {
                        if (CompareImageHashes(srcImageFile.FullName, destInfo.FullName))
                        {
                            Log("SKIPPED copy (Dup found) for: " + srcImageFile);
                            break;
                        }
                    }

                    ++i;
                }
            }
            else
            {
                File.Copy(srcImageFile.FullName, destPath);
            }
        }

        private bool CompareImageHashes(string srcImageFile, string destImageFile)
        {
            var shaProvider = new SHA1CryptoServiceProvider();

            var srcImageBytes = File.ReadAllBytes(srcImageFile);
            var srcHash = shaProvider.ComputeHash(srcImageBytes);

            var dstImageBytes = File.ReadAllBytes(destImageFile);
            var dstHash = shaProvider.ComputeHash(srcImageBytes);

            return srcHash.SequenceEqual(dstHash);
        }
        public void Log(String lines)
        {
            //Set the log file
            string logFile = destDirectory + "log.txt";

            File.AppendAllLines(logFile, new[] { lines });
        }

You've got moving, tagging, de-duping, fixing date stamp inconsistencies. You should know there are good free tools for that (and non-free tools like Lightroom) that are already written -- and more importantly -- already debugged.

Of course, if you're just looking for a code review, that's okay too.