Restore SQLite database backup

Question

I have an application that provides the user the ability to backup the local SQLite database to another location. I then provide the ability to restore the backup database and overwrite the current database with the backup copy. In my restore command, I make a backup of the database file, which seems redundant. I feel like this code could be simplified.

public async void RestoreDatabase()
{
 bool success;
 var pickedFile = default(StorageFile);
 var picker = new FileOpenPicker();
 picker.FileTypeFilter.Add(".sqlite");
 picker.SuggestedStartLocation = PickerLocationId.DocumentsLibrary;
 var file = await picker.PickSingleFileAsync();
 if (file != null)
 {
 var pickedFileToken = StorageApplicationPermissions.FutureAccessList.Add(file);
 _settings.RestoreFileToken = pickedFileToken;
 pickedFile = file;
 }
 // first make a backup of the current database file
 var currentDatabase = await _currentFolder.GetFileAsync("MyMoney.sqlite");
 if (currentDatabase != null)
 {
 currentDatabase.CopyAsync(ApplicationData.Current.RoamingFolder, "MyMoney.bak",
 NameCollisionOption.ReplaceExisting);
 }
 try
 {
 if (pickedFile != null)
 {
 await pickedFile.CopyAsync(ApplicationData.Current.RoamingFolder, "MyMoney.sqlite",
 NameCollisionOption.ReplaceExisting);
 success = true;
 }
 else
 {
 success = false;
 }
 }
 catch (Exception)
 {
 success = false;
 }
 if (!success)
 {
 // restore the backup file
 var backupDatabase = await _currentFolder.GetFileAsync("MyMoney.bak");
 if (backupDatabase != null)
 {
 await backupDatabase.CopyAsync(ApplicationData.Current.RoamingFolder, "MyMoney.sqlite",
 NameCollisionOption.ReplaceExisting);
 }
 }
}

• BCdotWEB: Please do not add an improved version of the code after receiving an answer.. "If you want to show everyone how you improved your code, but don't want to ask another question, then post an answer to your own question." (But please read the whole section before you do anything.)

Answer 1

I would suggest that you create some intermediate methods instead of doing everything in one method. For example the task of restoring a database is comprised of these subtasks:

• Select database file
• Create copy
• Restore database

---

var pickedFile = default(StorageFile);

It's a rather convoluted way to say IStorageFile pickedFile = null, don't you think? Personally I don't see many people use default() for this purpose so I would stick with the more common way of initializing.

---

await _currentFolder.GetFileAsync("MyMoney.sqlite");

Why are you hardcoding the filenames? This limits your re-use in case you want to expand it to other databases as well.

---

currentDatabase.CopyAsync(ApplicationData.Current.RoamingFolder, "MyMoney.bak", NameCollisionOption.ReplaceExisting);

This call is not awaited!

---

public async void RestoreDatabase()

Always avoid async void. From a recent answer of mine:

Asynchronous methods should return Task or Task<T>, the former being for traditional void methods and the latter being for methods with T return type.

The reason for this is that a void method cannot be awaited (since it doesn't return a Task) and will in effect be a fire-and-forget kind of thing: it will start executing your asynchronous code on a separate thread and continue doing business as normal. However once the main thread reaches its end, the application will exit regardless of whether or not your asynchronous code has finished as well. This obviously leads to nasty bugs.

In case you ever want to re-use this code for a console application or just in a class library where you don't know the client, I would suggest making it async Task just for good measure.

The only exception to this are asynchronous event handlers since they have to adhere to the convention that event handlers return void. This also explains why events are "dangerous" to use in asynchronous code.

---

RestoreDatabase()

Convention dictates that asynchronous methods should have the Async suffix.

• dub stylee: Thank you for your feedback. I will work on splitting the work out into different methods. As far as the async void - it is an event handler. This method is just fed into a DelegateCommand when a button in my View is clicked. As you say in your answer from the previous post, this is an exception to the rule. Please let me know if this is the wrong usage. Also, I will add Async to the method name, and added the await for the CopyAsync() call.
• dub stylee: I have added your suggestions to my original question. The only thing I wasn't able to do is parameterize the databaseName, since the method is passed into a DelegateCommand, which expects no parameters. Perhaps there is a better way to handle this scenario that will allow me to generalize things even more?
• Jeroen Vannevel: @dubstylee: I've rolled back your question since you cannot update it which invalidates answers. You are always free to post a new question though. I am not familiar with DelegateCommand myself so I can't help you there.