Skip to main content
Commonmark migration
Source Link

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.

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.

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.

Fix code formatting
Source Link
user34073
user34073

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");

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.

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.

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.

Source Link
Jeroen Vannevel
  • 11.6k
  • 2
  • 38
  • 79

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.