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.

I have a .net 4.5 desktop script which has dependencies on some third party libraries (dll's). These libraries are used in my MainWindowViewModel. I need to check that these dll's are present before their absence allow the program to break (which is basically at the starting point of the code). This is how I am handling it at the moment: wrapping the code which fires up the MainWindowViewModel's constructor inside a try/catch. Is this the right way to do this?

In my App.xaml.cs

    protected override void OnStartup(StartupEventArgs e)
    {
        base.OnStartup(e);

        MainWindow window = new MainWindow();

        //Create the ViewModel to which the main window binds.
        try
        {
            var viewModel = new MainWindowViewModel();
            // When the ViewModel asks to be closed, 
            // close the window.
            EventHandler handler = null;
            handler = delegate
            {
                viewModel.RequestClose -= handler;
                window.Close();
            };
            viewModel.RequestClose += handler;

            window.DataContext = viewModel;
            window.Show();
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
            window.Close();
        }
    }

If the dll is missing, running the program causes a message box to pop up (as you see in the code), and tells the user:

Could not load file or assembly 'xxx.net.xxxx, version = x.x.xx.x, culture ... or one of the dependencies. The system cannot find the file specified.

I would like to have a better message rather than the whole exception body in the message box. Maybe some dedicated method could return the dll name, version, etc. separately rather than me trying to clip it out of the exception.

share|improve this question
    
Where is the late binding happening? In the ViewModel? In the View? Is it supposed to be in this code and it's not written yet? As it stands your question isn't very clear. The right way to do what? To display a window? –  Mat's Mug Jun 13 at 0:27
    
I've read some stuff related to AssemblyResolve, however, from what this article was referring I couldn't figure out if this is related to my issue or how to implement it. It was called "AppDomain.AssemblyResolve Event Tips" from here codeproject.com/Articles/310675/… –  Mehrad Jun 13 at 0:28
    
I've added the info you wanted. And the question is, is this the right way to control if the required third party dll is accompanying the executable. –  Mehrad Jun 13 at 0:34
2  
@Mat'sMug In fact what I understood is this: if the dll is not installed, the code throw an exception. He open up a window with ex.Message that probably contain : Could not load file or assembly .... So I think this is his way of handling the missing dll. So the short answer to your question is no, since you do not directly control if the dll is present or not but just the side effect of it. –  Marc-Andre Jun 13 at 1:10
1  
I've retracted both the down and the close vote, since I ended up reviewing your code anyway.. but I still think your question would be much better if you included the "spot the missing dll" code. Cheers! (PS - welcome to CR, feel free to meet the site regulars anytime, in The 2nd Monitor). –  Mat's Mug Jun 13 at 1:42

2 Answers 2

up vote 6 down vote accepted

As I mentioned in the comments, the code you've posted could very well be doing anything that could potentially throw any exception.

MainWindow window = new MainWindow();

By putting this constructor call outside the try block, any exception thrown in the MainWindow constructor will be unhandled and the program will terminate in a not-so-gracious way. I assume the constructor does nothing special, but your OnStartup override is also making that assumption; I'd move the call inside the try block.

var viewModel = new MainWindowViewModel();

Your usage of var is inconsistent - I'd declare the MainWindow with var as well, but you could also declare the MainWindowViewModel instance with an explicit type; all that matters is consistency.

The fact that the ViewModel's constructor isn't taking any parameters tells me that the MainWindowViewModel class is likely to be tightly coupled with whatever dependencies it might have; the ViewModel could instead be constructor-injected with some service whose sole purpose would be to perform the 3rd-party DLL late binding - if that service has its own dependencies, for example a Log4net logger, it can receive it in its own constructor:

var logger = LogManager.GetLogger(typeof(LateBindingService))
var service = new LateBindingService(logger);
var viewModel = new MainWindowViewModel(service);

That would make the intent of your code much clearer, and would keep the ViewModel class more focused on being a ViewModel.

Then if you want to use an IoC container to automatically resolve and inject dependencies, you'll have absolutely nothing to change!


I like that you are holding a reference to your handler so it can be unregistered, but I find it would be cleaner to just register a private method instead of a delegate:

var view = new MainWindow();

var logger = LogManager.GetLogger(typeof(LateBindingService))
var service = new LateBindingService(logger);
var viewModel = new MainWindowViewModel(service);

viewModel.RequestClose += ViewModel_RequestClose;
view.DataContext = viewModel;
view.Show();

And that would be the whole try block.

The LateBindingService would be responsible for knowing everything there is to know about the 3rd party DLL, and would expose a method to try loading it.


The catch block should catch a much more specific exception type than System.Exception, since you want to show that message box if the late binding fails, not if anything else goes wrong at one point or another in the View's lifetime.

I'd probably have the LateBindingService throw some LateBindingException which exposes properties for everything there is to know about the missing assembly; the Message property would be 100% under your control, and then the catch block could be like this:

catch(LateBindingException exception)
{
    MessageBox.Show(exception.Message);
    viewModel.OnRequestClose();
}

Notice that I'm not calling Close directly on the view, because I want the handler to unregister itself before exiting, so I'd just have the ViewModel expose a method that raises the RequestClose event - convention for such methods is On[EventName], so OnRequestClose.

share|improve this answer
    
Very Descriptive and great answer. One addition. One of the DLLs I am using is for Log4Net and it's very simple one. All I needed to do is to add it to my references and create a var in the form of private static readonly ILog log = LogManager.GetLogger(typeof(MainWindowViewModel));. Would you be able to expand on your answer about how you're suggesting to move this process into your recommended var service = new LateBindingService(); ? –  Mehrad Jun 15 at 23:41
1  
@Mehrad edited; the constructor would have to be something like public LateBindingService(ILog logger) { _logger = logger; }, assigning a private readonly ILog _logger; field, and then _logger can be used as needed inside the LateBindingService class. The Ninject IoC container has a Ninject.Extensions.Logging namespace that supports Log4Net, you might want to look into that, too. –  Mat's Mug Jun 15 at 23:53

Why not use Assembly.Load (Reflection) to attempt to load the assemblies in question? You know what third party dependencies you have (because you built the app) and this list is fixed. Simply loop through these assemblies and if any do not bind, you have a cleaner name of the assembly to display and you can also have specific messages on how the user could resolve the problem (by installing dependency package or whatever).

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.