0

I wanted to create some simple practice code in UWP that opens up a text file, then immediately appends that code with trivial text.

public MainPage()
{
    this.InitializeComponent();

    OpenFile();
    SaveFile();
}

public async void OpenFile()
{
    FileOpenPicker picker = new FileOpenPicker();
    picker.SuggestedStartLocation = PickerLocationId.Desktop;
    picker.FileTypeFilter.Add(".txt");

    DataFile = await picker.PickSingleFileAsync();
    if (DataFile == null) { return; }
}

public async void SaveFile()
{
    await FileIO.AppendTextAsync(DataFile, "" + DateTime.Now + "\n");
}

private StorageFile DataFile { get; set; }

As expected, this code returns an error since in SaveFile() method, since SaveFile() runs immediately after OpenFile() and since OpenFile() has not completed its operation of retrieving the target file for SaveFile() to use. The thing is, when I try to modify the following code in OpenFile, I receive an AggregateException error:

DataFile = picker.PickSingleFileAsync();
Task task = Task.Run( async() => DataFile = await picker.PickSingleFileAsync() );
task.Wait();

I was wondering how I can block OpenFile() until it is done retrieving the target file, before SaveFile() runs.

Paul Taele
  • 51
  • 4
  • This can be solved by the answers in this question: https://stackoverflow.com/questions/32084943/async-lock-not-allowed – Laith Oct 07 '17 at 04:49
  • async composes poorly, it is turtles all the way down and the core problem here is that the final turtle, the constructor cannot be async. You just need to do this in reverse, *first* ask for a file name, *then* create the class object. That its name is MainPage is not great, never ask for user input immediately at program start before showing any UI. Dialogs need a window to stay on top of. Otherwise the reason why programs commonly have a File > Open command. – Hans Passant Oct 07 '17 at 07:20

1 Answers1

3

The constructor of a class should be fast and definitely shouldn't have any async methods. In your case you have been firing those methods as fire-forget, there is a race condition, where you haven't picked a file and you try to save to it. Nothing good comes from that.

Instead if you want a user to do soemthing once your MainPage is created, you may for example use Loaded event for this:

public MainPage()
{
    this.InitializeComponent();
    this.Loaded += LoadedHandler;
}

private async void LoadedHandler(object sender, RoutedEventArgs e)
{
    // assume that we want to do this only once
    this.Loaded -= LoadedHandler;
    await OpenFile();
    await SaveFile();
}

public async Task OpenFile()
{
    FileOpenPicker picker = new FileOpenPicker();
    picker.SuggestedStartLocation = PickerLocationId.Desktop;
    picker.FileTypeFilter.Add(".txt");

    DataFile = await picker.PickSingleFileAsync();
    if (DataFile == null) { return; }
}

public async Task SaveFile()
{
    await FileIO.AppendTextAsync(DataFile, "" + DateTime.Now + "\n");
}

private StorageFile DataFile { get; set; }

Note also that I've changed your methods to Tasks - you should avoid async void.

Romasz
  • 29,662
  • 13
  • 79
  • 154
  • 1
    Converting them to `Task` doesn't help by itself... you don't have the calls inside a `try / catch` and the caller (the event handler) is `void` so exceptions will still leak out into `Unobserved` exceptions. – Peter Torr - MSFT Oct 07 '17 at 23:36
  • @PeterTorr-MSFT You are right in this case. The provided link after edit, should give some light to the reader. I've added this to mention about possible problems. – Romasz Oct 08 '17 at 07:26