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.
Post History
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 sugges...
Answer
#1: Initial revision
Some thoughts: 0. 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. 1. 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 separate `Model` 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._ 2. `BaseViewModel`. * Use [`CallerMemberName`](https://docs.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.callermembernameattribute?view=net-5.0) attribute * Use new version of C# to simplify null's check. * To reduce boilerplate code you can add helpful `SetProperty` method ```csharp 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; } ``` 3. `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. 4. `CreateNewViewModel` * Use `camelCase` for fields. * Passing something as a command parameter despite you have direct access to it that's odd. ```csharp 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`) 5. `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.