debugcsharpMinor
Catching exception inside model
Viewed 0 times
catchingmodelexceptioninside
Problem
Is it normal to catch exception in concept of MVVM inside model? Or how should it be? How to improve this code?
This is simple application that allows user to drop .pfx certificate on the label.
Xaml:
Code:
```
namespace WpfApplication31
{
public partial class MainWindow : Window
{
public MainWindow()
{
InitializeComponent();
DataContext = new LoginViewModel();
}
}
public class LoginViewModel : ViewModelBase
{
LoginModel model = new LoginModel();
private string userCertificateResult = "place user certificate";
public string UserCertificateResult
{
get
{
return userCertificateResult;
}
set
{
userCertificateResult = value;
RaisePropertyChanged("UserCertificateResult");
}
}
public RelayCommand CreateConnectionCommand
{
get
{
return new RelayCommand(CreateConnection);
}
}
public RelayCommand DropUserCertificateCommand
{
get
{
return new RelayCommand(CheckUserCertificate);
}
}
private string userPassword = "123456";
public string UserPassword
{
get
{
return userPassword;
}
set
{
userPassword = value;
}
}
private X509Certificate2 UserCertificate;
private void CheckUserCertificate(DragEventArgs args)
{
try
This is simple application that allows user to drop .pfx certificate on the label.
LoginModel checks the password for validity and check the publisher. if the validation fails throws an exception that is catching in the LoginViewModel;Xaml:
Code:
```
namespace WpfApplication31
{
public partial class MainWindow : Window
{
public MainWindow()
{
InitializeComponent();
DataContext = new LoginViewModel();
}
}
public class LoginViewModel : ViewModelBase
{
LoginModel model = new LoginModel();
private string userCertificateResult = "place user certificate";
public string UserCertificateResult
{
get
{
return userCertificateResult;
}
set
{
userCertificateResult = value;
RaisePropertyChanged("UserCertificateResult");
}
}
public RelayCommand CreateConnectionCommand
{
get
{
return new RelayCommand(CreateConnection);
}
}
public RelayCommand DropUserCertificateCommand
{
get
{
return new RelayCommand(CheckUserCertificate);
}
}
private string userPassword = "123456";
public string UserPassword
{
get
{
return userPassword;
}
set
{
userPassword = value;
}
}
private X509Certificate2 UserCertificate;
private void CheckUserCertificate(DragEventArgs args)
{
try
Solution
There is nothing inherently wrong with catching exceptions in viewmodel. The rule of the thumb is that you catch exceptions at level where you can handle them. Sometimes this level is viewmodel level.
That being said, exceptions should not be used to control flow. This topic is covered on the Internet pretty well. For example, check this discussion or this MSDN guideline. Exceptions should be used for handling "exceptional" situations. There is nothing exceptional about certificate having a different issuer from the one you need. On the contrary, I would assume, this should be a pretty common situation.
So, instead of throwing exceptions - return a bool value. Might look like this:
or like this
or w/e really, there is plenty of options. Just don't throw exceptions.
Also this smells:
What exactly are you doing it for? Just let the original exception through, don't catch it, if you can't do anything about it.
That being said, exceptions should not be used to control flow. This topic is covered on the Internet pretty well. For example, check this discussion or this MSDN guideline. Exceptions should be used for handling "exceptional" situations. There is nothing exceptional about certificate having a different issuer from the one you need. On the contrary, I would assume, this should be a pretty common situation.
So, instead of throwing exceptions - return a bool value. Might look like this:
var certificate = model.LoadCertificate(userCertificatePath, UserPassword);
var isValid = model.IsCertificateValid(certificate);or like this
var isValid = model.TryLoadCertificate(userCertificatePath, UserPassword, out certificate);or w/e really, there is plenty of options. Just don't throw exceptions.
Also this smells:
catch (CryptographicException ex)
{
throw new CryptographicException(ex.Message.ToString());
}What exactly are you doing it for? Just let the original exception through, don't catch it, if you can't do anything about it.
Code Snippets
var certificate = model.LoadCertificate(userCertificatePath, UserPassword);
var isValid = model.IsCertificateValid(certificate);var isValid = model.TryLoadCertificate(userCertificatePath, UserPassword, out certificate);catch (CryptographicException ex)
{
throw new CryptographicException(ex.Message.ToString());
}Context
StackExchange Code Review Q#144769, answer score: 3
Revisions (0)
No revisions yet.