2

I feel something is wrong with my approach handling MVP and the Repository Pattern. I'm making an album winform app just to practice MVP, crud and the Repos. Pattern. First some code.

The model:

using System.ComponentModel.DataAnnotations;

namespace Models
{
    public class Album : IAlbum
    {
        [Key]
        public int ID { get; set; }

        [MinLength(1) ,StringLength(100), Required (ErrorMessage = "An Album Title is required (max 100 char).")]       
        public string Album_Title { get; set; } = null!;

        [MinLength(1), StringLength(100), Required(AllowEmptyStrings = false, 
            ErrorMessage = "An Artist Name is required (max 50 char).")]
        public string Artist { get; set; } = null!;

        [Required(ErrorMessage = "A Year (album release) is required")]
        [Range(1900, 2100, ErrorMessage = "The year must be between 1900 and 2200.")]
        public int Year { get; set; }

        [Url, Required(ErrorMessage = "An URL to an image of the Album is required.")]
        public string Image_URL { get; set; } = null!;

        [MinLength(20), Required(AllowEmptyStrings = false, 
            ErrorMessage = "A description of the Album is required with a minimum of 20 characters.")]
        public string Description { get; set; } = null!;

        //TODO: add a List<Track>() later.
    }
}

Now the DTO:

using System.ComponentModel.DataAnnotations;

namespace PresentationLayer.DTO
{
    public class AlbumDTO
    {
        [Key]
        public int ID { get; set; }

        [MinLength(1), StringLength(100), Required(ErrorMessage = "An Album Title is required (max 100 char).")]
        public string Album_Title { get; set; } = "";

        [MinLength(1), StringLength(100), Required(AllowEmptyStrings = false,
            ErrorMessage = "An Artist Name is required (max 50 char).")]
        public string Artist { get; set; } = "";

        [Required(ErrorMessage = "A Year (album release) is required")]
        [Range(1900, 2100, ErrorMessage = "The year must be between 1900 and 2200.")]
        public int Year { get; set; } = 0;

        [Url, Required(ErrorMessage = "An URL to an image of the Album is required.")]
        public string Image_URL { get; set; } = "";

        [MinLength(20), Required(AllowEmptyStrings = false,
            ErrorMessage = "A description of the Album is required with a minimum of 20 characters.")]
        public string Description { get; set; } = "";
    }
}

It's basically a copy of the model. Already I feel nervous. It's just copying the model in this case, i.e. double work. Here's a screenshot of the app I've made so far so you have an idea what's going on: Just basic CRUD setup.

enter image description here

The BindingSource for the DataGridView is set in the view's presenter:

private void SetBindingSource()
{
    _albumBindingSource.DataSource = _albumList;
    _mainView.SetAlbumsBindingSource(_albumBindingSource);
}

But now I'm a little bit in trouble as in the View (a WinForm) I cannot use efficient DataGridView methods such as:

DataGridView dgv = (DataGridView)sender;

if (dgv.CurrentRow != null)
{
    Album selectedAlbum = (Album)dgv.CurrentRow.DataBoundItem;

    lbl_ID.Text = selectedAlbum.ID.ToString();
}

Because the view isn't suppose to know about the model. And the conversion to aforementioned AlbumDTO class fails. Maybe I could try adjusting the model like:

public class Album
{
    public int ID { get; set; }
    public string Album_Title { get; set; } = "";
    public string Artist { get; set; } = "";
    public int Year { get; set; } = 0;
    public string Image_URL { get; set; } = "";
    public string Description { get; set; } = "";

    public Album(AlbumDTO albumDTO)
    {
        ID = albumDTO.ID;
        Album_Title = albumDTO.Album_Title;
        Artist = albumDTO.Artist;
        Year = albumDTO.Year;
        Image_URL = albumDTO.Image_URL;
        Description = albumDTO.Description;
    }
}

But now the model is aware of the AlbumDTO. The point, as far as I know, is that the "model" doesn't know anybody. So I don't like this.

Question: To me the question arises whether I designed all of this right to begin with and simply use the AlbumDTO class whilst using alternative ways with the DataGridView such as:

DataGridView dgv = (DataGridView)sender;

    if (dgv.CurrentRow != null)
    {
        int rowClicked = dgv.CurrentRow.Index;

        // Retrieve data from the selected row based on the index
        string albumTitle = dgv.Rows[rowClicked].Cells["AlbumTitleColumn"].Value.ToString();
        string artist = dgv.Rows[rowClicked].Cells["ArtistColumn"].Value.ToString();
        string year = dgv.Rows[rowClicked].Cells["YearColumn"].Value.ToString();
//...etc...

The point is to load GridView data into edit boxes to edit data and then update the database. So what do you say design wise? Did I miss the mark somewhere here?

(Note, I'm an hobbyist learning).

9
  • 2
    It's basically a copy of the model. Yes sometimes it feel boring to rewrite the same classe. For me THE necessary DTO is the DTO that crossing the boundary beetween the model and the database. So after that your view only work with the ModelDTO (and not the DatabaseDTO). After that you can pass the ModelDTO to the view. And the view use it
    – Adri
    Commented Jun 9, 2023 at 12:29
  • 1
    With special HTML comments. The one I used for you post: <!-- language: lang-csharp --> Commented Jun 9, 2023 at 12:43
  • 1
    There is a link to a detailed help page about how to use markdown on this site when you are on the edit screen for your question. Commented Jun 9, 2023 at 12:44
  • 1
    Why do you have a DTO? The MVP doesn't require it and your code doesn't seems to need it either.
    – Laiv
    Commented Jun 10, 2023 at 7:40
  • 1
    DTOs are widely misused. if you limit DTOs to M.F's few use cases, you won't be wrong
    – Laiv
    Commented Jun 10, 2023 at 17:08

1 Answer 1

1

Disclaimer: I wanted the OP to write and check the answer, but at the time of this writing he hasn't enough reputation.

OP and I came to the conclusion that, for this specific case DTOs are unnecessary complexity. The MVP provide enough decoupling between view and model.

DTO is a pattern widely misused, often than not, due to guides and tutorials of doubtful credibility.

In my experience, DTOs fits perfectly in the use cases described by Martin Fowler P of EAA : DTO Pattern and rarely when they are addressed to decouple layers belonging to a single boundary.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.