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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design of Library #3

Open
nedimf opened this issue Aug 11, 2019 · 5 comments
Open

Design of Library #3

nedimf opened this issue Aug 11, 2019 · 5 comments
Assignees
Labels
design Feedback from people who'd want to use the library

Comments

@nedimf
Copy link
Owner

nedimf commented Aug 11, 2019

Please share your feedback if you want to use this library and help us grow 馃帀
We strive for improvement so any feedback even slightest is always welcome.

@nedimf nedimf added the design Feedback from people who'd want to use the library label Aug 11, 2019
@nedimf nedimf self-assigned this Aug 11, 2019
@javier-moreno
Copy link
Contributor

javier-moreno commented Aug 12, 2019

Builder Pattern

The Builder pattern is the best option for this.

The main issue being:
Data members of class aren't guaranteed to be initialized.

Because of this a few of the required fields might not be initialized, for example:

  • smtp
  • smtpUsername
  • smtpPassword
  • smtpAuthentication
  • port

If any of those fields is not initialized the email won't be sent. I'm not saying that the builder approach should be completely removed, but it need to be changed to have some required fields and some optional fields. I have some ideas on how to approach this, let me know if you are interested on this improvement.

@javier-moreno
Copy link
Contributor

SuccessCallback Design

The current successCallback implementation needs to be improved, right now the way it is done it behaves in the following way.

  1. The callback gets set through onCompleteCallback with a timeout.
  2. After being set Handler().postDelayed(...) gets called with the specified timeout.
  3. When the timeout expires the code gets executed regardless of the rest of whatever happened with the builder and the sending of the email.

This is why this is not a good approach:

  • If for some reason if the sending process of the email takes more than timeout, onCompleteCallback is going to be called and a fail result is going to be called simply because mailSuccess was never set.
  • If the setting up of the builder is done in multiple steps that require different user inputs through different screens the timeout is going to expire certainly before everything is done and set.
  • Right now we are tied to the timeout to know whats the result, even if the process of sending the email is blazing fast we still need to wait for the arbitrary timeout we setup to know the result.

A callback shouldn't be tied to some arbitrary timeout, callback are expected to be called when the process is ended or some error occurred .

Approach

Run the callback at the end of the execution or when an error happens.

@nedimf
Copy link
Owner Author

nedimf commented Aug 12, 2019

Amazing, you did great analysis. I agree with everything said. 馃憤

@nedimf
Copy link
Owner Author

nedimf commented Aug 13, 2019

If I send a message to a bad address, why don't I get a SendFailedException or TransportEvent indicating that the address is bad?

@nedimf
Copy link
Owner Author

nedimf commented Aug 13, 2019

There is no end-to-end address verification on the Internet. Often a message will need to be forwarded to several mail servers before reaching one that can determine whether or not it can deliver the message. If a failure occurs in one of these later steps, the message will typically be returned to the sender as undeliverable. A successful "send" indicates only that the mail server has accepted the message and will try to deliver it.

From javamail-api

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Feedback from people who'd want to use the library
Projects
No open projects
Development

No branches or pull requests

2 participants