Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reuse TodoItem class #18

Closed
nbarbettini opened this issue Oct 23, 2017 · 7 comments
Closed

Reuse TodoItem class #18

nbarbettini opened this issue Oct 23, 2017 · 7 comments
Milestone

Comments

@nbarbettini
Copy link
Owner

Is the NewTodoItem POCO an unnecessary duplication of TodoItem?

My initial thought was keeping everything super clean and separate, but that might be a little too much.

@nbarbettini nbarbettini added this to the 2.0 milestone Jan 15, 2018
@sadqiang
Copy link

sadqiang commented Jan 17, 2018

Probably you can reuse TodoItem but with Bind attribute (to selectively choose properties to be bound) when explaining the model binding concept. NewTodoItem with just one property Title of type string seems to be overkill in general but a good and simpler example for explaining model binding that populates a class object without Bind property.

@mattwelke
Copy link

As far as I can tell, the NewTodoItem is a good example of a Dto (aka view model, resource model, etc). I really like that design pattern, so I looked seeing it in this book. But I can see how that would have seemed a bit unclear to beginner programmers.

@nbarbettini
Copy link
Owner Author

@Welkie Yeah, that's exactly what I was trying to illustrate (DTOs). This example didn't come out as clean as I wanted because the properties are identical to TodoItem, but I'll see if it makes sense to keep it.

@nbarbettini
Copy link
Owner Author

@sadqiang I agree, it would be cool to show model binding - feels like it might be outside the scope for now though. In a more complex application model binding could be very useful.

@mattwelke
Copy link

mattwelke commented Jan 19, 2018

@nbarbettini I really like DTOs, I use them for add, edit, delete, command use cases, etc. But they do add a lot of code that beginners (like me, before I took that class in school) can have trouble understanding. It feels like fluff to them, layers in between without purpose. I think that this book would be best without them, just using the model classes for changing data in the database.

I think DTOs aren't really an ASP.NET Core thing, they're a web development in general thing, and that they would be a great entry in that "extra" category we discussed adding in previous issues, along with subjects like AJAX, SPAs, etc.

Might you be interested in a pull request for a chapter on them now that my school is dying down and I have some time to make meaningful contributions to the book? :)

@sadqiang
Copy link

sadqiang commented Jan 20, 2018

In my perspective if NewTodoItem is still kept there for the next release of the book, it can also be regarded as a good introduction to model binding. You don't need to say much about it if it is beyond the scope, but saying something like

Hey, look there is an interesting feature, the form field Title submitted to the server gets wrapped by NewTodoItem as an argument of public async Task<IActionResult> AddItem(NewTodoItem newItem). Title of type string is converted or serialized automatically by the framework model binding to NewTodoItem. This feature is good if the form has many fields submitted to the server because a single action method argument suffices to accept a bunch of incoming form fields.

Of course DTO is another good aspect!

@nbarbettini nbarbettini modified the milestones: 2.0, 1.1 Apr 6, 2018
@nbarbettini
Copy link
Owner Author

What I decided to do in fbddcae was:

  • Get rid of NewTodoItem and reuse TodoItem
  • Add a paragraph explaining the DTO concept as an alternative
  • Talk a little bit more about model binding

Thanks to both of you for the feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants