2
\$\begingroup\$

I got this background console script running, which pulls all necessary XML files from the server and stores them locally for faster performance. These XML files contain Graphs full of data, which will be shown inside the application.

Right now the user can get to the graphs in about 45 seconds if he directly goes towards them. And that is just about enough time for the files to download before the user can reach them. But as years go by, more graphs will become available and they might not be done downloading by the time the user gets there.

Is it possible to speed up the process of fetching those files? Comments on naming and readability are also appreciated.

using System;
using System.Collections.Generic;

namespace MonavisaBackGroundLoading
{
    // Request form class to log into monavisa

    public class MonavisaRequestForm
    {
        #region variables
        public readonly string username;
        public readonly string password;
        public string url;
        public System.Net.WebClient webclient;
        #endregion

        public MonavisaRequestForm(string username, string password, string url, ref System.Net.WebClient webclient)
        {
            this.username = username;
            this.password = password;
            this.url = url;
            this.webclient = webclient;
        }
    }

    class Program
    {
        static System.DateTime dt = System.DateTime.Now;
        static bool firstToTrigger = false;

        static void Main(string[] args)
        {

            //Check if Pre-Fetch has been run before, if so the application will keep this in mind and re-load the last month
            //So that this data is fully up to date
            string[] text = null;
            if(System.IO.File.Exists(string.Format(@"{0}\Nioo Graph Data {1}.xml", "C:/Users/LAB2-Computer/Desktop/nioo 2.0/NIOO V3.1", "last pull date as registerd by apllication.txt")))
                text = System.IO.File.ReadAllLines(string.Format(@"{0}\Nioo Graph Data {1}.xml", "C:/Users/LAB2-Computer/Desktop/nioo 2.0/NIOO V3.1", "last pull date as registerd by apllication.txt"));
            Console.WriteLine(text);
            System.Threading.Thread.Sleep(5000);

            //Start download of files, from start year till current year. 
            Console.WriteLine("Initiating downloading XML");
            for(var i = 2013; i <= dt.Year; i++)
            {
                Console.WriteLine(i);
                for (var ii = 0; ii < 12; ii++)
                {
                    string dateAndTime;
                    //Modified string for the file name
                    string fileDateAndTime;

                    //As the date presumes a end date we have to set 12 to 1 so that the 12th month gets pulled
                    //For the rest we just need to do +2
                    if (ii == 12)
                    {
                        dateAndTime = string.Format("{0}-{1}-{2}+12:00", 01, 01, i);
                        fileDateAndTime = string.Format("{0}-{1}-{2}", 01, 01, i);
                    }
                    else
                    {
                        dateAndTime = string.Format("{0}-{1}-{2}+12:00", 01, ii + 2, i);
                        fileDateAndTime = string.Format("{0}-{1}-{2}", 01, ii + 1, i);
                    }

                    //Check if the file already exists
                    if (System.IO.File.Exists(string.Format(@"{0}\Nioo Graph Data {1}.xml", System.IO.Directory.GetParent(AppDomain.CurrentDomain.BaseDirectory).ToString(), fileDateAndTime))) 
                    {
                        //If it exists but is the last downloaded in the previous download, download it again to update that month
                        if (text != null)
                        {
                            if (i == Convert.ToInt32(text[1]) && ii == Convert.ToInt32(text[0]))
                            {
                                CreateRequest(dateAndTime, fileDateAndTime);
                                continue;
                            }
                            else continue;
                        }
                        continue; 
                    }
                    else
                    {
                        //File does not exist, create a download request.
                        CreateRequest(dateAndTime, fileDateAndTime);
                        continue;
                    }
                }
            }

            //Update the previous download sesion with the latest date. 
            string[] lines = {dt.Month.ToString(),dt.Year.ToString(),"End of program"};

            System.IO.File.WriteAllLines(string.Format(@"{0}\Nioo Graph Data {1}.xml", System.IO.Directory.GetParent(System.IO.Directory.GetParent(AppDomain.CurrentDomain.BaseDirectory).ToString()).ToString(), "last pull date as registerd by apllication"), lines);
        }

        //Create a login request for monavisa with login info for the date requested
        private static System.Net.WebClient myWebClient = new System.Net.WebClient();
        private static void CreateRequest(string dateAndTime, string fileDateAndTime)
        {
            MonavisaRequestForm myRequest = new MonavisaRequestForm
            (
                "foo",
                "bar",
                string.Format("http://www.monavisa.info/CreateGraphData?graphs=1&graph[0]=1226&todate={0}&period={1}&step={2}&b_id=194&inter=1&other_graph=false", dateAndTime, 3, 1),
                ref myWebClient
            );
            MonavisaFetch.instance.PreObtainData(ref myRequest, dateAndTime, fileDateAndTime);
        }
    }

    public class MonavisaFetch 
    {
        private static MonavisaFetch fetchmonavisa;
        private MonavisaFetch() { }
        public static MonavisaFetch instance
        { 
            get{
                if (fetchmonavisa == null){
                    fetchmonavisa = new MonavisaFetch();
                }
                return fetchmonavisa; 
            } 
        }

        private Queue<MonavisaRequestForm> requestQueue = new Queue<MonavisaRequestForm>();
        private System.Timers.Timer timer = new System.Timers.Timer(3000);
        private bool initialized = false;

        //Start a timer for the fetch
        public void initialize()
        {
            timer.AutoReset = true;
            timer.Elapsed += new System.Timers.ElapsedEventHandler(onTimerElapsed);
            timer.Start();
        }

        //If timer ends qeue form
        private void onTimerElapsed(object sender, System.Timers.ElapsedEventArgs args)
        {
            if (requestQueue.Count > 0)
            {
                MonavisaRequestForm tmpRequest = requestQueue.Peek();
                GetData(ref tmpRequest);
            }
        }

        public void GetData(ref MonavisaRequestForm request)
        {
            if (!initialized)
                initialize();
            //Fetch the document using local php login 
            try
            {
                if (request.username != null)
                {

                    if (!request.webclient.IsBusy && requestQueue.Count == 0)
                    {
                        request.url = request.url.Replace("&", "%26");
                        request.url = request.url.Replace("+", "%2B");
                        Uri uri = new Uri(string.Format("http://localhost/login.php?username={0}&password={1}&request={2}", request.username, request.password, request.url));
                        request.webclient.DownloadStringAsync(uri);

                    }
                    else if (!request.webclient.IsBusy && requestQueue.Count > 0)
                    {
                        Uri uri = new Uri(string.Format("http://localhost/login.php?username={0}&password={1}&request={2}", requestQueue.Peek().username, requestQueue.Peek().password, requestQueue.Peek().url));
                        requestQueue.Peek().webclient.DownloadStringAsync(uri);
                        requestQueue.Dequeue();
                    }
                    else
                    {
                        requestQueue.Enqueue(request);
                    }
                }
                else
                {
                    Uri uri = new Uri(request.url);
                    request.webclient.DownloadStringAsync(uri);
                }
            }
            catch (System.Net.WebException ex)
            {
                if (ex.Status != System.Net.WebExceptionStatus.ProtocolError)
                {
                    throw;
                }
            }
        }   

        public void PreObtainData(ref MonavisaRequestForm request, string dateAndTime, string fileDateAndTime)
        {
            if (!initialized)
                initialize();
            try
            {
                if (!request.webclient.IsBusy && requestQueue.Count == 0)
                {
                    request.url = request.url.Replace("&", "%26");
                    request.url = request.url.Replace("+", "%2B");
                    Uri uri = new Uri(string.Format("http://localhost/login.php?username={0}&password={1}&request={2}", request.username, request.password, request.url));
                    request.webclient.DownloadFile(uri, @"Nioo Graph Data " + fileDateAndTime + ".xml"); 
                }
                else if (!request.webclient.IsBusy && requestQueue.Count > 0)
                {
                    Uri uri = new Uri(string.Format("http://localhost/login.php?username={0}&password={1}&request={2}", requestQueue.Peek().username, requestQueue.Peek().password, requestQueue.Peek().url));
                    requestQueue.Peek().webclient.DownloadStringAsync(uri);
                    requestQueue.Dequeue();
                }
                else
                {
                    requestQueue.Enqueue(request);
                }
            }
            catch (System.Net.WebException ex)
            {
                if (ex.Status != System.Net.WebExceptionStatus.ProtocolError)
                {
                    throw ;
                }
            }
        }
    }
}
\$\endgroup\$
3
\$\begingroup\$

If you call continue; in an if block the else isn't needed any more. So this

if (text != null)
{
    if (i == Convert.ToInt32(text[1]) && ii == Convert.ToInt32(text[0]))
    {
        CreateRequest(dateAndTime, fileDateAndTime);
        continue;
    }
    else continue;
}
continue;  

could be replaced by

if (text != null)
{
    if (i == Convert.ToInt32(text[1]) && ii == Convert.ToInt32(text[0]))
    {
        CreateRequest(dateAndTime, fileDateAndTime);
    }
}
continue;  

or much better if we see the whole if..else beast

//Check if the file already exists
if (System.IO.File.Exists(string.Format(@"{0}\Nioo Graph Data {1}.xml", System.IO.Directory.GetParent(AppDomain.CurrentDomain.BaseDirectory).ToString(), fileDateAndTime))) 
{
    //If it exists but is the last downloaded in the previous download, download it again to update that month
    if (text != null)
    {
        if (i == Convert.ToInt32(text[1]) && ii == Convert.ToInt32(text[0]))
        {
            CreateRequest(dateAndTime, fileDateAndTime);
        }
    }
}
else
{
    //File does not exist, create a download request.
    CreateRequest(dateAndTime, fileDateAndTime);
}

This

string dateAndTime;
//Modified string for the file name
string fileDateAndTime;

//As the date presumes a end date we have to set 12 to 1 so that the 12th month gets pulled
//For the rest we just need to do +2
if (ii == 12)
{
    dateAndTime = string.Format("{0}-{1}-{2}+12:00", 01, 01, i);
    fileDateAndTime = string.Format("{0}-{1}-{2}", 01, 01, i);
}
else
{
    dateAndTime = string.Format("{0}-{1}-{2}+12:00", 01, ii + 2, i);
    fileDateAndTime = string.Format("{0}-{1}-{2}", 01, ii + 1, i);
}  

can be replaced by

string dateAndTime = string.Format("{0}-{1}-{2}+12:00", 01, ii + 2, i);
string fileDateAndTime = string.Format("{0}-{1}-{2}", 01, ii + 1, i);

because ii will never be 12

Basically you should for optimization purpose

  • store results of the same operation inside variables like Convert.ToInt32(text[1])
  • avoid checks where the result won't change like if (text != null)

So your main() method can be simplified to

static void Main(string[] args)
{
    int year = -1;
    int month = -1;
    string[] text = null;

    if (System.IO.File.Exists(string.Format(@"{0}\Nioo Graph Data {1}.xml", "C:/Users/LAB2-Computer/Desktop/nioo 2.0/NIOO V3.1", "last pull date as registerd by apllication.txt")))
    {
        text = System.IO.File.ReadAllLines(string.Format(@"{0}\Nioo Graph Data {1}.xml", "C:/Users/LAB2-Computer/Desktop/nioo 2.0/NIOO V3.1", "last pull date as registerd by apllication.txt"));
        if (text != null && text.Length == 3)
        {
            month = Convert.ToInt32(text[0]);
            year = Convert.ToInt32(text[1]);
        }
    }

    String directory = System.IO.Directory.GetParent(AppDomain.CurrentDomain.BaseDirectory).ToString();

    for (var i = 2013; i <= dt.Year; i++)
    {
        Console.WriteLine(i);
        for (var ii = 0; ii < 12; ii++)
        {
            string dateAndTime = string.Format("{0}-{1}-{2}+12:00", 01, ii + 2, i);
            string fileDateAndTime = string.Format("{0}-{1}-{2}", 01, ii + 1, i);
            string fileName = string.Format(@"{0}\Nioo Graph Data {1}.xml", directory, fileDateAndTime);

            if (!System.IO.File.Exists(fileName) || (i == year && ii == month))
            {
                CreateRequest(dateAndTime, fileDateAndTime);
            }
        }
    }

    string[] lines = { dt.Month.ToString(), dt.Year.ToString(), "End of program" };
    System.IO.File.WriteAllLines(string.Format(@"{0}\Nioo Graph Data {1}.xml", directory, "last pull date as registerd by apllication"), lines);
}
\$\endgroup\$
4
\$\begingroup\$

Some simple stuff for now:

  1. Standard naming convention in C# for property names is PascalCase.

  2. Why are you using string.Format to format the file name when the parts are all static strings? Unless I'm missing something then this:

    string.Format(@"{0}\Nioo Graph Data {1}.xml", "C:/Users/LAB2-Computer/Desktop/nioo 2.0/NIOO V3.1", "last pull date as registerd by apllication.txt")
    

    can be replaced with

    const string fileName = @"C:/Users/LAB2-Computer/Desktop/nioo 2.0/NIOO V3.1/Nioo Graph Data last pull date as registerd by apllication.txt";
    

    Also you are performing the string.Format twice repeating the string parts which is prone to cut-n-paste errors and/or typos. You should at least store the result in a local variable.

  3. What's the point of sleeping 5 seconds after loading the file into memory? Seems like wasting time.

  4. While it's accepted practice to name simple loop counters with single letter variables in your case year for the outer loop and month for the inner loop would be more appropriate as that's what their meaning is.

  5. Looks like your are putting all requests in a queue which downloads all files 1-by-1. If the server permits you could look at downloading the files in parallel to speed things up.

\$\endgroup\$
  • \$\begingroup\$ Nice points, especially the second. That was a bug, it was supposed to be the base directory. Thank you for pointing that out \$\endgroup\$ – MX D Nov 12 '14 at 9:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.