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.
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).
1 Answer 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.
Explore related questions
See similar questions with these tags.
<!-- language: lang-csharp -->