search

jozefizso / SystemWrapper

.NET library for easier testing of system APIs.
Microsoft Public License
175 stars 62 forks source link

Updated SmtpClientWrap #8

Closed rhyous closed 8 years ago

rhyous commented 8 years ago

Hey,

I am one of the developers on the original CodePlex site. As the CodePlex site is pretty much dead, I was going to fork it on GitHub today and found you already have, which is awesome. :+1:

The reason this came up today is that I have a new and more complete ISmptClient and SmtpClientWrap that I was going add to the project. However, my implementation calls MailMessage directly.

I notice in this fork, interfaces and wrappers were added for these objects that SmtpClientWrap currently uses:

MailMessage = IMailMessage, MailMessageWrap MailAddress = IMailAddress, MailAddressWrap MailAddressCollection = IMailAddressCollection, IMailAddressFactoryWrap MailAddressFactory = IMailAddressFactory, IMailAddressFactoryWrap

I hesitate to say that someone waisted their time because I am not 100% sure they did. However, MailMessage, MailAddress, MailAddressCollection are all little more than poco classes and as such, they would fall into the category of objects that Should not be wrapped.

In a unit test, a MailMessage, MailAddress, MailAddressCollection can be created and used no problem. The original project intent was to provide interfaces and wrapping things that cannot be created and used in Unit Tests otherwise. These objects can.

Is there something hidden in these objects that required them to be wrapped? Or is there another use case that suggests they Should be wrapped?

If not, I will submit a pull request with my new SmtpClientWrap and remove the unnecessary wrapping of these poco classes.

Obviously someone added these interfaces and before I submit a pull request to remove them I would like to verify whether they are really needed or not.

jozefizso commented 8 years ago

Hi rhyous,

those classes were added in this pull request: https://github.com/jozefizso/SystemWrapper/pull/5

I did not do any review of those classes. I think they were automatically generated.

If they don't provide any meaningful benefit, I would suggest to remove them to simplify the API.

rhyous commented 8 years ago

Great.

jozefizso commented 8 years ago

Released in v0.10.0

rhyous commented 8 years ago

Thank you. I have removed SmtpWrap from my source and am now using your NuGet packages in QA. It was great to find this project so much further along in GitHub than I expected.

jozefizso commented 8 years ago

You are welcome.