Android - Running endless code in Foreground while using the app

Question

Im making an application that i want to have the ability to send report emails with a frequency. daily, weekly or monthly.

Now i want those emails to be sent only if the app is running and if the user is using it (in my app you use the mobile as a kiosk, you open the app and leave it running all the time, screen will not close)

I have write this code in the MainActivity where if someone has enabled the frequently email feature, the activity will try to send the email (SendStatisticsToEmailBySchedule() will run forever and will use SendStatisticsToEmail() from time to time to send an email):

public async void SendStatisticsToEmail(string subject, string address, int port, string from, string to, bool useSSL, bool saveDateSent = false, string username = "", string password = "", bool sendNowButtonUsed = false)
        {
            var fileName = System.IO.Path.Combine(
                GetString(Resource.String.Statistics_ExportAllExportPrefix,
                firstResult.ToString("MM-dd-yyyy"),
                DateTime.Today.ToString("MM-dd-yyyy")));

            var message = new MimeMessage();
            message.From.Add(new MailboxAddress(from));
            message.To.Add(new MailboxAddress(to));
            message.Subject = subject + " " + fileName;

            var statistics = new Statistics();
            var reportsForXML = statistics.ConvertRecordsForExport(MyData);
            MemoryStream stream = new MemoryStream();

            var builder = new BodyBuilder();
            var image = BitmapFactory.DecodeResource(Resources, Resource.Drawable.get_started);
            byte[] imgbyte;
            using (var bitmap = image)
            {
                var streamByte = new MemoryStream();
                bitmap.Compress(Bitmap.CompressFormat.Png, 100, streamByte);
                bitmap.Recycle();
                imgbyte = streamByte.ToArray();
            }
            var imageAttach = builder.LinkedResources.Add(@"get_started.png", imgbyte);
            imageAttach.ContentId = MimeUtils.GenerateMessageId();
            builder.HtmlBody = string.Format(@"<p align=""center""><center><img height=""150"" width=""328"" src=""cid:{0}""></center></p><p align=""center"">{1}</p><p align=""center"">{2}</p><p align=""center"">{3}</p>", new object[] { imageAttach.ContentId, GetString(Resource.String.EmailExportSettings_MessageBodyLine1), GetString(Resource.String.EmailExportSettings_MessageBodyLine2), GetString(Resource.String.EmailExportSettings_MessageBodyLine3) });

            using (System.IO.StreamWriter writer = new System.IO.StreamWriter(stream))
            {
                reportsForXML.WriteXml(writer, XmlWriteMode.WriteSchema, false);
                stream.Position = 0;
                builder.Attachments.Add(fileName, stream);
            }
            message.Body = builder.ToMessageBody();

            try
            {
                using (var client = new SmtpClient())
                {
                    // For demo-purposes, accept all SSL certificates (in case the server supports STARTTLS)
                    client.ServerCertificateValidationCallback = (s, c, h, e) => true;

                    if (useSSL)
                        await client.ConnectAsync(address, port, SecureSocketOptions.SslOnConnect);
                    else
                        await client.ConnectAsync(address, port, false);

                    // Note: only needed if the SMTP server requires authentication
                    if (!string.IsNullOrEmpty(username) && !string.IsNullOrEmpty(password))
                        client.Authenticate(username, password);

                    await client.SendAsync(message);
                    await client.DisconnectAsync(true);
                    if (saveDateSent)
                    {
                        Preferences.Set(Settings.ExportToEmailLastEmailSentKey, DateTime.Now);
                        ScheduleNextEmailForSend(DateTime.Now,
                            Preferences.Get(Settings.ExportToEmailEmailFrequencyPositionKey, Settings.ExportToEmailEmailFrequencyPositionDefault)
                            );
                    }
                    if (sendNowButtonUsed)
                        Toast.MakeText(this, GetString(Resource.String.Statistics_EmailSendSuccess), ToastLength.Long).Show();
                }
            }
            catch //(Exception exception)
            {
                if (sendNowButtonUsed)
                    Toast.MakeText(this, GetString(Resource.String.Statistics_EmailSendFailure), ToastLength.Long).Show();
            }
        }
private void ScheduleNextEmailForSend(DateTime nextScheduledEmailDate, int frequency)
    {
        nextScheduledEmailDate = DateTime.Now;
        frequency = Preferences.Get(Settings.ExportToEmailEmailFrequencyPositionKey, Settings.ExportToEmailEmailFrequencyPositionDefault);
        switch (frequency)
        {

            case 0:
                Preferences.Set(Settings.ExportToEmailNextEmailForKey, nextScheduledEmailDate.AddDays(1));
                break;
            case 1:
                Preferences.Set(Settings.ExportToEmailNextEmailForKey, nextScheduledEmailDate.AddDays(7));
                break;
            case 2:
                Preferences.Set(Settings.ExportToEmailNextEmailForKey, nextScheduledEmailDate.AddDays(30));
                break;
        }
    }

This the the code that will run forever

private async void SendStatisticsToEmailBySchedule()
        {
            do
            {
                if (MyData.Count != 0)
                {
                    var currentConnectivity = Connectivity.NetworkAccess;
                    NextScheduledDateForReport = Preferences.Get(Settings.ExportToEmailNextEmailForKey, CurrentDateSettings.ExportToEmailNextEmailForDefault);

                    if (DateTime.Now > NextScheduledDateForReport)
                    {
                        if (currentConnectivity == NetworkAccess.Internet)
                        {
                            try
                            {
                                SendStatisticsToEmail(Preferences.Get(Settings.ExportToEmailSubjectKey, Settings.ExportToEmailSubjectDefault),
                                    Preferences.Get(Settings.ExportToEmailServerAddressKey, Settings.ExportToEmailServerAddressDefault),
                                    Preferences.Get(Settings.ExportToEmailPortKey, Settings.ExportToEmailPortDefault),
                                    Preferences.Get(Settings.ExportToEmailFromKey, Settings.ExportToEmailFromDefault),
                                    Preferences.Get(Settings.ExportToEmailToKey, Settings.ExportToEmailToDefault),
                                    Preferences.Get(Settings.ExportToEmailUseSSLKey, Settings.ExportToEmailUseSSLDefault),
                                    true,
                                    Preferences.Get(Settings.ExportToEmailUsernameKey, Settings.ExportToEmailUsernameDefault),
                                    Preferences.Get(Settings.ExportToEmailPasswordKey, Settings.ExportToEmailPasswordDefault)
                                );
                            }
                            catch
                            {

                            }
                        }
                    }
                }
                await Task.Delay(21600);
            } while (true);
        }

From what i have read here https://developer.android.com/guide/components/services#Choosing-service-thread if you want to run some code only while the user is using your app, you should not use a service.

Is my code on SendStatisticsToEmailBySchedule() efficient?

Is there any situation that will stop working or should i somehow check that it is still working?

Answer 1

It's better to use a System.Timers.Timer for this job and the best way is to create it on OnCreate, pause it on OnPause and resume it on OnResume

protected override void OnCreate()
{
if(Timer_Is_Needed)
   SendStatisticsToEmailBySchedule();
}

protected override void OnResume()
        {
            StartSendStatisticsToEmailByScheduleTimer();
}

private void SendStatisticsToEmailBySchedule()
        {
            SendStatisticsToEmailByScheduleTimer = new System.Timers.Timer
            {
                Interval = 3600000,
                Enabled = true,
                AutoReset = true,
            };
            SendStatisticsToEmailByScheduleTimer.Elapsed +=
                new System.Timers.ElapsedEventHandler(OnSendStatisticsToEmailByScheduleTimerEvent);
            SendStatisticsToEmailByScheduleTimer.Start();
        }

        private void StopSendStatisticsToEmailByScheduleTimer()
        {
            if (SendStatisticsToEmailByScheduleTimer != null)
                if (SendStatisticsToEmailByScheduleTimer.Enabled == true)
                    SendStatisticsToEmailByScheduleTimer.Stop();
        }

        private void StartSendStatisticsToEmailByScheduleTimer()
        {
            if (SendStatisticsToEmailByScheduleTimer != null)
                if (SendStatisticsToEmailByScheduleTimer.Enabled == false)
                    SendStatisticsToEmailBySchedule();
        }
        protected override void OnPause()
        {
            StopSendStatisticsToEmailByScheduleTimer();
        }

Answer 2

Some quick remarks:

• SendStatisticsToEmail() is a 75 line method with almost a dozen arguments and it does waaay too much. Split this method up into smaller methods -- for instance: one that gets the fileName, one that composes the body, etc. -- that do one single thing, and instead of passing so many arguments, pass a single class that has all these arguments as properties.

  (See for instance https://softwareengineering.stackexchange.com/q/27798/123339 on the "ideal" method length. Sure, this is personal and debatable, but once you need to scroll to read through the contents of a method you're probably on the wrong path and you'll need to refactor things.)

  Case in point: your method contains ten lines that are dedicated to converting an image into bytes. This should clearly be a method of its own instead of being chucked in the middle of this overlong mess. Another example: on line 17 you say var builder = new BodyBuilder(); and then you don't use builder until line 27. This indicates a severe lack of structure and program flow.

  Even better: consider converting SendStatisticsToEmail() into a class with one public method and several private ones, instead of it being one method among many. You might even consider converting some of those private methods into classes of their own.

• One of the lines in SendStatisticsToEmail() is 450 characters long. This is unreadable and impossible to maintain. Even if you were to introduce some line-breaks, it would still be far too hard to maintain. Also, your email's body should really contain better HTML than what you currently offer.

  Instead, consider moving your email template to an embedded HTML file, with its variables to be filled in replaced by placeholders like _ContentId_. You read the embedded file into a string, and then have a method that loops through a Dictionary<string, string> where the keys are those placeholders and the values are the actual values.

• Why is the if (saveDateSent) logic inside the using (var client = new SmtpClient()) block? Ditto if (sendNowButtonUsed). These do not belong there. The whole using (var client = new SmtpClient()) should be a method or even a class of its own, and that should return a bool to indicate success or failure, and that bool should then be used to trigger the logic the if (saveDateSent) logic and the if (sendNowButtonUsed) logic (and the UI logic now residing in the catch block).

• Avoid mixing UI elements into methods that should stay free of them. Sure, you need to report back on the success of sending an email to the user, but not from inside a convoluted method. Things like sending emails, calling the API etc. should be part of a service layer.