Re: Update script for 329841 finished. Comments?

[Ruben Vermeersch <ruben Lambda1 be>]

On Thu, 2006-02-16 at 00:45 +1100, Bengt Thuree wrote:
> Hi Gabriel,
> 
> Just finished my small patch for the Updater.cs to add a leading 0 to
> the Month and Day directory.
> 
> And as you requested.
> try
> 	1) Create Directory
> 	2) hard link photo
> 	3) update database
> 	4) move thumbnails
> catch
> 	revert thumbnails
> 	remove new hard link
> 	remove new directory
> 
> remove old hard link
> remove old directories
> 
> 
> The patch for FileImporterBacken.cs is just 2 lines.
> The patch for Updater.cs is 333 lines!!!!
> Guess I need a refresher course in C#, or rather a beginner course
> 
> Would appreciate some comments if anyone have any.
> 
> Do bear in mind that please test this on a test system, not on your live
> data. Also, just after 4 (in the above pseudo code) you can uncomment a
> raise exception so no changes will be made...

Personally, I'd like this patch, just reading through it now:

You iterate all pictures by getting the min id and the max id:

	for (int id = min_photo_id;  id <= max_photo_id; id++) {

Does this cope with deleted pictures? Why not just do a "SELECT COUNT(*)
FROM photos;", then return if needed. Afterwards, select all pictures
and iterate. If memory usage is the problem, select all ids and then
iterate.

Though it's not functional programming, why not use continue instead of
huge if's (making most of the code drop 2 indents):

	if (orig_directory.StartsWith (FSpot.Global.PhotoDirectory)) {
		*huge if-else block*
will become:
	if (!orig_directory.StartsWith (FSpot.Global.PhotoDirectory)) {
		Console.WriteLine ("----> {0} ----< Directory ({1}) is outside
~/Photos ignore it.", id, orig_directory);
		continue;
	}

Similar:

	if ((photo_time.Month < 10) || (photo_time.Day < 10)) {
becomes:
	if ((photo_time.Month > 10) && (photo_time.Day > 10)) {
		Console.WriteLine ("----> {0} ----< directory {1} - Month {2} - Day
{3} is OK.", id, orig_directory, photo_time.Day, photo_time.Month);
		continue;
	}

Spelling-police: ;-)
- throw new Exception (String.Format("Photo with name {0} do not
exists", old_photo));
+ throw new Exception (String.Format("Photo with name {0} does not
exist", old_photo));

Most of this is just style. I assume it doesn't delete directories that
still have files in them?

Do like this patch, having the missing leading zero's is just plain
annoying.

Kind regards,
	Ruben


--
Ruben Vermeersch (rubenv)
http://www.Lambda1.be/