Skip to main content
Code Review

Return to Revisions

2 of 2
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/

Looks pretty good. Just a few little things:

  • Command-related methods CanDoXXXX and DoXXXX would be better off named CanExecuteXXXX and ExecuteXXXX or OnExecuteXXXX.
  • I would put the ViewModel properties IsEditable and IsDirty in some IEditable interface that applicable ViewModels implement. Looks like reusable stuff.
  • Your VM is tightly coupled with your log provider - I'd wrap it with some ILogger interface which would be constructor-injected as needed.
  • You're not always consistent with access modifiers: sometimes you do specify the default (private void InitialiseProduct), other times you leave it (void VM_PropertyChanged). I'd specify it everywhere (be explicit).
  • IApplicationManager is dangerous. You're passing it as a dependency which is good, but its name (xxxxManager) suggests that it might, over time, become a shove-it-all-in-there helper class with low cohesion. You don't show much code, but if all it's used for in the VM is to get the app.CurrentUser, maybe the VM's dependencies are more with some ICurrentUser than the whole IApplicationManager.
  • Your ViewModelBase doesn't show its INotifyPropertyChanged implementation, I'd do something like this so instead of OnPropertyChanged("CoreEditorVM") you could do OnPropertyChanged(() => CoreEditorVM) and have a strongly-typed way of referring to your property names.
Mathieu Guindon
  • 75.5k
  • 18
  • 194
  • 467
default

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