current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

I'm using a DataContext to insert an object. Currently it's working, but I'm feeling things could be done better. Method is called from UI (Using Caliburn Micro Binding). IsBusy is a property used by the UI (Xceed Extended WPF Toolkit) to display a wait message.

Repository inherits from DataContext (using Entity Code First).

public async void AddTemplate()
{
    IsBusy = true;

    await Task.Run(() => {
        using (var repo = new Repository())
        {
            var template = new Template() { ... };
            try
            {
                repo.Templates.Add(template);
                repo.SaveChanges();
                this.TryClose(true);
            }
            catch (SqlException ex)
            {
                _util.Log.AddException(Exceptions.DB_EXC, ex.Message);
            }
            catch (EntityException ex)
            {
                _util.Log.AddException(Exceptions.EF_EXC, ex.Message);
            }
            catch (Exception ex)
            {
                _util.Log.AddException(Exceptions.US_EXC, ex.Message);
            }
            finally
            {
                IsBusy = false;
            }
        }
        this.TryClose(false);
    });
}

I'm feeling odd about covering using with the await Task.Run since I can also await for repo.SaveChangesAsync(), but then IsBusy will not change (and the Wait message will not be made visible).

share|improve this question

1 Answer 1

up vote 3 down vote accepted

You should be able to await the repo.SaveChangesAsync() method.

public async Task AddTemplateAsync()
{
    var template = new Template {...};

    using (var repo = new Repository())
    {
        try
        {
            IsBusy = true;
            repo.Templates.Add(template);
            await repo.SaveChangesAsync();
        }
        catch {...}
        finally
        {
            IsBusy = false;
            TryClose();
        }
    }
}

Since AddTemplate() is an async method, it should be called AddTemplateAsync() and return a Task. Take a look at these best practices for more details. The calling code should not await the Task returned from AddTemplateAsync() so as not to block the UI thread. You might want to consider passing in a CancellationToken depending on your requirements.

I would also advise against swallowing the base Exception type for reasons explained here.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.