Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

Picking up some of what Mat's Mug Mat's Mug left

clDelegateCommand

public event EventHandler CanExecuteChanged
{
 add { CommandManager.RequerySuggested += value; }
 remove { CommandManager.RequerySuggested -= value; }
}

I use to call CommandManager.RequerySuggested -= value; before I add it. Doing this won't lead to any problems, if the same value hasn't been subscibed to the eventhandler, but it ensures that it isn't subscribed twice. Assume you already subscribed to the handler and do it again, you will receive the same event 2 times.

 public event EventHandler CanExecuteChanged
 {
 add 
 { 
 CommandManager.RequerySuggested -= value; 
 CommandManager.RequerySuggested += value; 
 }
 remove { CommandManager.RequerySuggested -= value; }
 }

clBaseAddEditDeleteViewModel and clPart

Here a check is missing for the properties changed

public bool IsListEnabled
{
 get { return this._IsListEnabled; }
 set
 {
 this._IsListEnabled = value;
 OnPropertyChanged("IsListEnabled");
 }
}

The event is raised each time the property is set. It should be only raised if the property is changed instead.

public bool IsListEnabled
{
 get { return this._IsListEnabled; }
 set
 {
 if (this._IsListEnabled == value) {return;}
 this._IsListEnabled = value;
 OnPropertyChanged("IsListEnabled");
 }
}

Check all of your properties.

Style

Sometimes you use braces {} for single if statements, sometimes you don't.
Be consequent on the style you choose. Many prefer to use always braces for single if statement or single lined for loops, so do I.

Picking up some of what Mat's Mug left

clDelegateCommand

public event EventHandler CanExecuteChanged
{
 add { CommandManager.RequerySuggested += value; }
 remove { CommandManager.RequerySuggested -= value; }
}

I use to call CommandManager.RequerySuggested -= value; before I add it. Doing this won't lead to any problems, if the same value hasn't been subscibed to the eventhandler, but it ensures that it isn't subscribed twice. Assume you already subscribed to the handler and do it again, you will receive the same event 2 times.

 public event EventHandler CanExecuteChanged
 {
 add 
 { 
 CommandManager.RequerySuggested -= value; 
 CommandManager.RequerySuggested += value; 
 }
 remove { CommandManager.RequerySuggested -= value; }
 }

clBaseAddEditDeleteViewModel and clPart

Here a check is missing for the properties changed

public bool IsListEnabled
{
 get { return this._IsListEnabled; }
 set
 {
 this._IsListEnabled = value;
 OnPropertyChanged("IsListEnabled");
 }
}

The event is raised each time the property is set. It should be only raised if the property is changed instead.

public bool IsListEnabled
{
 get { return this._IsListEnabled; }
 set
 {
 if (this._IsListEnabled == value) {return;}
 this._IsListEnabled = value;
 OnPropertyChanged("IsListEnabled");
 }
}

Check all of your properties.

Style

Sometimes you use braces {} for single if statements, sometimes you don't.
Be consequent on the style you choose. Many prefer to use always braces for single if statement or single lined for loops, so do I.

Picking up some of what Mat's Mug left

clDelegateCommand

public event EventHandler CanExecuteChanged
{
 add { CommandManager.RequerySuggested += value; }
 remove { CommandManager.RequerySuggested -= value; }
}

I use to call CommandManager.RequerySuggested -= value; before I add it. Doing this won't lead to any problems, if the same value hasn't been subscibed to the eventhandler, but it ensures that it isn't subscribed twice. Assume you already subscribed to the handler and do it again, you will receive the same event 2 times.

 public event EventHandler CanExecuteChanged
 {
 add 
 { 
 CommandManager.RequerySuggested -= value; 
 CommandManager.RequerySuggested += value; 
 }
 remove { CommandManager.RequerySuggested -= value; }
 }

clBaseAddEditDeleteViewModel and clPart

Here a check is missing for the properties changed

public bool IsListEnabled
{
 get { return this._IsListEnabled; }
 set
 {
 this._IsListEnabled = value;
 OnPropertyChanged("IsListEnabled");
 }
}

The event is raised each time the property is set. It should be only raised if the property is changed instead.

public bool IsListEnabled
{
 get { return this._IsListEnabled; }
 set
 {
 if (this._IsListEnabled == value) {return;}
 this._IsListEnabled = value;
 OnPropertyChanged("IsListEnabled");
 }
}

Check all of your properties.

Style

Sometimes you use braces {} for single if statements, sometimes you don't.
Be consequent on the style you choose. Many prefer to use always braces for single if statement or single lined for loops, so do I.

Source Link
Heslacher
  • 50.9k
  • 5
  • 83
  • 177

Picking up some of what Mat's Mug left

clDelegateCommand

public event EventHandler CanExecuteChanged
{
 add { CommandManager.RequerySuggested += value; }
 remove { CommandManager.RequerySuggested -= value; }
}

I use to call CommandManager.RequerySuggested -= value; before I add it. Doing this won't lead to any problems, if the same value hasn't been subscibed to the eventhandler, but it ensures that it isn't subscribed twice. Assume you already subscribed to the handler and do it again, you will receive the same event 2 times.

 public event EventHandler CanExecuteChanged
 {
 add 
 { 
 CommandManager.RequerySuggested -= value; 
 CommandManager.RequerySuggested += value; 
 }
 remove { CommandManager.RequerySuggested -= value; }
 }

clBaseAddEditDeleteViewModel and clPart

Here a check is missing for the properties changed

public bool IsListEnabled
{
 get { return this._IsListEnabled; }
 set
 {
 this._IsListEnabled = value;
 OnPropertyChanged("IsListEnabled");
 }
}

The event is raised each time the property is set. It should be only raised if the property is changed instead.

public bool IsListEnabled
{
 get { return this._IsListEnabled; }
 set
 {
 if (this._IsListEnabled == value) {return;}
 this._IsListEnabled = value;
 OnPropertyChanged("IsListEnabled");
 }
}

Check all of your properties.

Style

Sometimes you use braces {} for single if statements, sometimes you don't.
Be consequent on the style you choose. Many prefer to use always braces for single if statement or single lined for loops, so do I.

default

AltStyle によって変換されたページ (->オリジナル) /