Communities

Writing
Writing
Codidact Meta
Codidact Meta
The Great Outdoors
The Great Outdoors
Photography & Video
Photography & Video
Scientific Speculation
Scientific Speculation
Cooking
Cooking
Electrical Engineering
Electrical Engineering
Judaism
Judaism
Languages & Linguistics
Languages & Linguistics
Software Development
Software Development
Mathematics
Mathematics
Christianity
Christianity
Code Golf
Code Golf
Music
Music
Physics
Physics
Linux Systems
Linux Systems
Power Users
Power Users
Tabletop RPGs
Tabletop RPGs
Community Proposals
Community Proposals
tag:snake search within a tag
answers:0 unanswered questions
user:xxxx search by author id
score:0.5 posts with 0.5+ score
"snake oil" exact phrase
votes:4 posts with 4+ votes
created:<1w created < 1 week ago
post_type:xxxx type of post
Search help
Notifications
Mark all as read See all your notifications »
Code Reviews

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

71%
+3 −0
Code Reviews C# WPF MVVM View & Get new record values

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...

posted 3y ago by FoggyFinder‭

Answer
#1: Initial revision by user avatar FoggyFinder‭ · 2021-02-02T14:46:41Z (about 3 years ago)
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.