Welcome to Software Development on Codidact!
Will you help us build our independent community of developers helping developers? We're small and trying to grow. We welcome questions about all aspects of software development, from design to code to QA and more. Got questions? Got answers? Got code you'd like someone to review? Please join us.
C# WPF MVVM View & Get new record values
Just looking to hear some reviews on my current MVVM Implementation, If I am heading towards the right direction. :)
Code
BaseViewModel
public class BaseViewModel : INotifyPropertyChanged
{
public event PropertyChangedEventHandler PropertyChanged;
protected void OnPropertyChanged(string propertyName)
{
if (PropertyChanged != null)
{
PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
}
}
}
ViewModel
public class Technical_BestBeforeDates_ViewModel : BaseViewModel
{
// Loading + Filtering Data
private string _ProductCode;
public string ProductCode
{
get { return _ProductCode; }
set
{
_ProductCode = value;
OnPropertyChanged(nameof(ProductCode));
UpdateDataFromDatabase();
}
}
private string _ProductDescription;
public string ProductDescription
{
get { return _ProductDescription; }
set
{
_ProductDescription = value;
OnPropertyChanged(nameof(ProductDescription));
UpdateDataFromDatabase();
}
}
public ObservableCollection<Technical_BestBeforeDates_Model> BestBeforeDates_Models { get; }
public void UpdateDataFromDatabase()
{
BestBeforeDates_Models.Clear();
var records = Technical_BestBeforeData.GetFilteredData(ProductCode, ProductDescription);
foreach (Technical_BestBeforeDates_Model model in records)
{
BestBeforeDates_Models.Add(model);
}
}
// Open New Window Command
public ICommand OpenCreateNewRecordCommand { get; set; }
public void OpenWindow()
{
Technical_BestBeforeDates_CreateNewView window = new Technical_BestBeforeDates_CreateNewView()
{
WindowStartupLocation = System.Windows.WindowStartupLocation.CenterScreen,
DataContext = new Technical_BestBeforeDates_CreateNewViewModel()
};
window.ShowDialog();
}
// Constructor
public Technical_BestBeforeDates_ViewModel()
{
BestBeforeDates_Models = new ObservableCollection<Technical_BestBeforeDates_Model>();
UpdateDataFromDatabase();
OpenCreateNewRecordCommand = new RelayCommand(_ => OpenWindow());
}
}
CreateNewViewModel
public class Technical_BestBeforeDates_CreateNewViewModel : BaseViewModel
{
private Technical_BestBeforeDates_Model _NewModel_;
public Technical_BestBeforeDates_Model NewModel
{
get { return _NewModel_; }
set
{
_NewModel_ = value;
OnPropertyChanged(nameof(NewModel));
}
}
private string _ProductCode;
public string ProductCode
{
get { return _ProductCode; }
set
{
_ProductCode = value;
NewModel = new Technical_BestBeforeDates_Model
{
ProductCode = _ProductCode,
ProductDescription = this.ProductDescription
};
OnPropertyChanged(nameof(ProductCode));
}
}
private string _ProductDescription;
public string ProductDescription
{
get { return _ProductDescription; }
set
{
_ProductDescription = value;
NewModel = new Technical_BestBeforeDates_Model
{
ProductCode = this.ProductCode,
ProductDescription = _ProductDescription
};
OnPropertyChanged(nameof(ProductDescription));
}
}
public ICommand CreateNewRecordCommand { get; set; }
public void CreateNewRecord(object param)
{
var vm = param as Technical_BestBeforeDates_Model;
System.Windows.MessageBox.Show(vm.ProductCode);
}
public Technical_BestBeforeDates_CreateNewViewModel()
{
CreateNewRecordCommand = new RelayCommand(param => CreateNewRecord(param));
}
}
RelayCommand
public class RelayCommand : ICommand
{
private Action<object> execute;
private Predicate<object> canExecute;
private event EventHandler CanExecuteChangedInternal;
public RelayCommand(Action<object> execute)
: this(execute, DefaultCanExecute)
{
}
public RelayCommand(Action<object> execute, Predicate<object> canExecute)
{
if (execute == null)
{
throw new ArgumentNullException("execute");
}
if (canExecute == null)
{
throw new ArgumentNullException("canExecute");
}
this.execute = execute;
this.canExecute = canExecute;
}
public event EventHandler CanExecuteChanged
{
add
{
CommandManager.RequerySuggested += value;
this.CanExecuteChangedInternal += value;
}
remove
{
CommandManager.RequerySuggested -= value;
this.CanExecuteChangedInternal -= value;
}
}
public bool CanExecute(object parameter)
{
return this.canExecute != null && this.canExecute(parameter);
}
public void Execute(object parameter)
{
this.execute(parameter);
}
public void OnCanExecuteChanged()
{
EventHandler handler = this.CanExecuteChangedInternal;
if (handler != null)
{
handler.Invoke(this, EventArgs.Empty);
}
}
public void Destroy()
{
this.canExecute = _ => false;
this.execute = _ => { return; };
}
private static bool DefaultCanExecute(object parameter)
{
return true;
}
}
Data Storage - Technical_BestBeforeData
public static class Technical_BestBeforeData
{
private static List<Technical_BestBeforeDates_Model> GetData()
{
using (MySqlConnection conn = new MySqlConnection(ConfigurationManager.ConnectionStrings["MySQL"].ConnectionString))
{
conn.Open();
string query = @"SELECT b.id AS Id,
ProductCode,
ProductDescription,
b.AreaID as AreaID,
MinimumMonths,
LAST_DAY(DATE_ADD(NOW(), INTERVAL MinimumMonths MONTH)) AS BestBeforeDate
FROM technical_bestbeforedates b"; ;
var records = conn.Query<Technical_BestBeforeDates_Model>(query).ToList();
return records;
}
}
private static bool Filter(Technical_BestBeforeDates_Model tbm, string code, string description)
{
return (string.IsNullOrEmpty(code) || tbm.ProductCode.Contains(code)) && (string.IsNullOrEmpty(description) || tbm.ProductDescription.Contains(description));
}
public static IEnumerable<Technical_BestBeforeDates_Model> GetFilteredData(string code, string description)
{
return GetData().Where(tbm => Filter(tbm, code, description));
}
}
Model
public class Technical_BestBeforeDates_Model
{
public int Id { get; set; }
public string ProductCode { get; set; }
public string ProductDescription { get; set; }
}
1 answer
Some thoughts:
-
Consider using Github (or something like that, but I suggest Github due to its popularity) to share the project. It allows us not only to write some code but open PR with suggestions.
-
Do not use such names as
Technical_BestBeforeDates_Model
, etc. Using of_
is against C# style guide.PascalCase
is a preferable way.
Besides, it is hard to perceive.
I suppose, for Technical_BestBeforeDates_Model
class you probably could:
- Use
Technical
as a name for namespace. - Remove
Model
part. Put it to a separateModel
project to stress its purpose.
So Technical_BestBeforeDates_Model
turns to Model.Technical.BestBeforeDates
.
In short:
Name should be laconic and, at the same time, meaningfull.
-
BaseViewModel
.
- Use
CallerMemberName
attribute - Use new version of C# to simplify null's check.
- To reduce boilerplate code you can add helpful
SetProperty
method
protected virtual bool SetProperty<T>(ref T storage, T value, [CallerMemberName] string propertyName = "")
{
if (EqualityComparer<T>.Default.Equals(storage, value))
{
return false;
}
storage = value;
this.OnPropertyChanged(propertyName);
return true;
}
Technical_BestBeforeDates_ViewModel
-
VM should know nothing about View. Use services or consider using SPA (single page application) approach to navigate between pages.
-
Be careful with sending a query to DB each time user changes something in a field. You could use
Delay
,Debounce
, etc to prevent spamming. But maybe that's not the case here.
CreateNewViewModel
-
Use
camelCase
for fields. -
Passing something as a command parameter despite you have direct access to it that's odd.
public class Technical_BestBeforeDates_CreateNewViewModel : BaseViewModel
{
private string productCode;
public string ProductCode
{
get { return productCode; }
set { SetProperty(ref productCode, value); }
}
private string productDescription;
public string ProductDescription
{
get { return productDescription; }
set { SetProperty(ref productDescription, value); }
}
public ICommand CreateNewRecordCommand { get; set; }
public void CreateNewRecord()
{
var m = new Technical_BestBeforeDates_Model
{
ProductCode = productCode,
ProductDescription = this.ProductDescription
};
System.Windows.MessageBox.Show(m.ProductCode);
}
public Technical_BestBeforeDates_CreateNewViewModel()
{
CreateNewRecordCommand = new RelayCommand(_ => CreateNewRecord());
}
}
- Again, there should be no reference to any type that related to View (like
MessageBox
)
Technical_BestBeforeData
- Name of this class doesn't reflect meaning.
- It would be better to filter data on a database side. It's usually faster there.
- Name of
GetData
method also doesn't reflect meaning. - You could move an sql query out of a method to keep them all together.
0 comment threads