HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Helper class to send emails with attachments on Android

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
sendandroidhelperwithemailsattachmentsclass

Problem

I use this EmailTools helper class inside an Android app to send data as an email attachment, by calling send(context, deviceName, recordedContent), where the parameters are:

-
context: an instance of Context, in order to:

  • create Intent.ACTION_SEND for launching an existing email app to send emails



  • get elements from string.xml to format the subject



  • get the app's version to include in the message body



  • deviceName: a String, the name of the device where the data is coming from.



  • recordedContent: a String, the recorded data, to send as attachment.



Is there a better way? How would you improve this?

```
public abstract class EmailTools {

private static final String TAG = EmailTools.class.getSimpleName();

private static final String MESSAGE_TYPE = "message/rfc822";

public static void send(Context context, String deviceName, String recordedContent) {
String subject = String.format(context.getString(R.string.fmt_subject_recorded_data), deviceName);
String message = String.format(context.getString(R.string.fmt_recorded_from), deviceName);
message += getPackageInfo(context);

Intent intent = new Intent(Intent.ACTION_SEND);
intent.setType(MESSAGE_TYPE);
intent.putExtra(Intent.EXTRA_SUBJECT, subject);
intent.putExtra(Intent.EXTRA_TEXT, message);
addAttachmentToIntent(context, deviceName, recordedContent, intent);
launchEmailApp(context, intent);
}

private static String getPackageInfo(Context context) {
String packageName = context.getPackageName();
PackageManager manager = context.getPackageManager();
try {
PackageInfo info;
if (manager != null) {
info = manager.getPackageInfo(packageName, 0);
if (info != null) {
return String.format("\n\n--\n[App: %s Version: %d/%s]",
packageName, info.versionCode, info.versionName);

Solution

I'd change a few things to make your code cleaner.

Your send method does three different things: it extract the message from the context, it creates the intent and it finally launches the email app. What about introducing a Message class, a Message decodeMessage(Context context, String deviceName), and an Intent createIntent(Message message) methods?

getPackageInfo is definitely to arrow-shaped. I'd refactor it to separate the logic of creation of the package info string String formatPackageInfo(PackageInfo packageInfo) and the code to retrieve the PackageInfo instance.

private PackageInfo getPackageInfo(){
    String packageName = context.getPackageName();
    PackageManager manager = context.getPackageManager();
    if (manager == null)
        return null;
    try
    {
        return manager.getPackageInfo(packageName, 0);
    }
    catch (PackageManager.NameNotFoundException e)
    {
        Log.e(TAG, "Could not get package info", e);
    }
    return null;
}

private static String formatPackageInfo(PackageInfo packageInfo) {
        if(packageInfo == null)
            return "";
        return String.format("\n\n--\n[App: %s Version: %d/%s]",
                        packageName, info.versionCode, info.versionName);
}


In addAttachmentToIntent I'd probably move the declaration of dateFormat and of filename in the try block. You're not going to use them outside that scope so it could be a good idea to give them the smallest scope possible.

Code Snippets

private PackageInfo getPackageInfo(){
    String packageName = context.getPackageName();
    PackageManager manager = context.getPackageManager();
    if (manager == null)
        return null;
    try
    {
        return manager.getPackageInfo(packageName, 0);
    }
    catch (PackageManager.NameNotFoundException e)
    {
        Log.e(TAG, "Could not get package info", e);
    }
    return null;
}

private static String formatPackageInfo(PackageInfo packageInfo) {
        if(packageInfo == null)
            return "";
        return String.format("\n\n--\n[App: %s Version: %d/%s]",
                        packageName, info.versionCode, info.versionName);
}

Context

StackExchange Code Review Q#63413, answer score: 3

Revisions (0)

No revisions yet.