Avatar billede hellfishdk Nybegynder
05. juni 2010 - 20:39 Der er 15 kommentarer og
2 løsninger

vurdering/optimering af kode

Hej

Jeg vil gerne have en vurdering/hjælp til optimering af min kode. Jeg synes at det virker lidt voldsomt, at min applikation bruger ca. 6 mb hukommelse, når den ikke skal udfører tunge opgaver.

Applikationen er Winforms app.

Kode er som følger:

Klasse til liste med SQL servers:

using System;
using System.Collections.Generic;
using System.ServiceProcess;
using Microsoft.Win32;
using System.Text;

namespace SQLCommander
{
    class SQLList
    {
        public List<ServiceController> SQLsList()
        {
            List<ServiceController> listOfSQL = new List<ServiceController>();

            //Get services
            ServiceController[] services = ServiceController.GetServices();

            List<ServiceController> mssqlList = new List<ServiceController>();
            List<ServiceController> mysqlList = new List<ServiceController>();
            List<ServiceController> oracleList = new List<ServiceController>();

            foreach (ServiceController scTemp in services)
            {
                //get MSSQL servers
                if (scTemp.ServiceName.Contains("MSSQL$"))
                {
                    mssqlList.Add(scTemp);
                }
            }

            //Get MySQL servers
            foreach (string name in getMySQLd())
            {
                foreach (ServiceController scTemp in services)
                {
                    if(scTemp.ServiceName.Contains(name))
                        mysqlList.Add(scTemp);
                }
            }
           

            //Get ORACLEXE servers
            foreach (ServiceController scTemp in services)
            {
                //get MSSQL servers
                if (scTemp.ServiceName.Contains("OracleService"))
                {
                    oracleList.Add(scTemp);
                }
            }

            services = null;

            //Add found sql server to collection
            //mssqls
            foreach (ServiceController scTemp in mssqlList)
                listOfSQL.Add(scTemp);

            //mysqls
            foreach (ServiceController scTemp in mysqlList)
                listOfSQL.Add(scTemp);

            //oracle
            foreach (ServiceController scTemp in oracleList)
                listOfSQL.Add(scTemp);

            //reset lists
            mysqlList.Clear();
            mysqlList = null;
            mssqlList.Clear();
            mssqlList = null;
            oracleList.Clear();
            oracleList = null;

            return listOfSQL;
        }

        private List<string> getMySQLd()
        {
            List<string> displayName = new List<string>();
            string[] st = Registry.LocalMachine.OpenSubKey(
            "System\\CurrentControlSet\\Services").GetSubKeyNames();
            StringBuilder sb = new StringBuilder();
            foreach (string key in st)
            {

                object imagePath;
                object DisplayName;

                if ((imagePath = Registry.LocalMachine.OpenSubKey(
                "System\\CurrentControlSet\\Services\\" + key).GetValue("ImagePath"))
                == null)
                    continue;

                if ((DisplayName = Registry.LocalMachine.OpenSubKey(
                "System\\CurrentControlSet\\Services\\" + key).GetValue("DisplayName"))
                == null)
                    DisplayName = "Unknow";

                if (imagePath.ToString().Contains("mysqld"))
                {
                    displayName.Add(DisplayName.ToString());
                }
                DisplayName = null;
                imagePath = null;
            }
            st = null;

            return displayName;
        }
    }
}



Form1.cs:
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Xml;
using System.Windows.Forms;
using System.ServiceProcess;

namespace SQLServerCommander
{
    public partial class Form1 : Form
    {
        private SQLList sl = new SQLList();
        private Languages languages = new Languages();
        private string language = "dansk";
        string serviceName = null;
       
        public Form1()
        {
            //getLanguage();
            InitializeComponent();
            initComponents();
            initMenuItems();
           
        }

        private void setLanguage(string lang)
        {
            XmlTextWriter xtw = new XmlTextWriter("Language.xml", null);
            xtw.WriteStartElement("Language");
            xtw.WriteString(lang);
        }

        private void getLanguage()
        {
            XmlDocument doc = new XmlDocument();
            doc.Load("Language.xml");
            XmlElement root = doc.DocumentElement;
            XmlNodeList nodes = root.SelectNodes("/Languages");

            foreach (XmlNode node in nodes)
            {
                language = node["Language"].InnerText;
            }
        }

        private void initComponents()
        {
            bindingComboBox();
            setLblSelectServerText(language);
            setBtnStartText(language);
            setBtnStopText(language);
            setBtnRestartText(language);
        }

        private void initMenuItems()
        {
            //Toplevel menuitems
            languagesToolStripMenuItem.Text = languages.languagesStripText(language);
            filesToolStripMenuItem.Text = languages.filesStripText(language);
            helpToolStripMenuItem.Text = languages.helpStripText(language);

            //Files menuitem
            exitToolStripMenuItem.Text = languages.exitStripText(language);

            //Languages menuitems
            englishToolStripMenuItem.Text = languages.englishStripText(language);
            danishToolStripMenuItem.Text = languages.danishStripText(language);

            //Help menuitems
            aboutToolStripMenuItem.Text = languages.aboutStripText(language);
        }

        private void Form1_Load(object sender, EventArgs e)
        {
           
        }

        private void bindingComboBox()
        {
                     
            List<string> listToBind = new List<string>();
            BindingList<string> bList = new BindingList<string>();

            int ln = 0;

            foreach (ServiceController str in sl.SQLsList())
            {
                string scName = str.ServiceName.ToString();
                if (scName.Length > ln)
                    ln = scName.Length;
                bList.Add(scName);
            }
            comboBox1.Width = ln*9;
            comboBox1.DataSource = bList;

            serviceName = comboBox1.SelectedValue.ToString();

            serviceStatus(serviceName);
        }

        private void serviceStatus(string sn)
        {
            string status = null;
            foreach (ServiceController sc in sl.SQLsList())
            {
                string name = sc.ServiceName.ToString();
                if (name.Equals(sn))
                {
                    status = sc.Status.ToString();
                }
            }

            if (status.Equals("Running"))
            {
                btn_Start.Enabled = false;
                btn_Stop.Enabled = true;
                btn_Restart.Enabled = true;
            }

            if(status.Equals("Stopped"))
            {
                btn_Start.Enabled = true;
                btn_Stop.Enabled = false;
                btn_Restart.Enabled = false;
            }
            //return status;
        }

        private void setLblSelectServerText(string lang)
        {
            lbl_SelectServer.Text = languages.lblSelectServer(lang);
        }

        private void setBtnStartText(string lang)
        {
            btn_Start.Text = languages.btnStart(lang);

           
        }

        private void setBtnStopText(string lang)
        {
            btn_Stop.Text = languages.btnStop(lang);
        }

        private void setBtnRestartText(string lang)
        {
            btn_Restart.Text = languages.btnRestart(lang);
        }

        private void comboBox1_SelectedIndexChanged(object sender, EventArgs e)
        {
            serviceName = comboBox1.SelectedValue.ToString();
            serviceStatus(serviceName);
        }

        private void btn_Start_Click(object sender, EventArgs e)
        {
            foreach (ServiceController sc in sl.SQLsList())
            {
                string name = sc.ServiceName.ToString();
                if (name.Equals(serviceName))
                {
                    sc.Start();
                    sc.WaitForStatus(ServiceControllerStatus.Running);
                    serviceStatus(serviceName);
                    initiateTimer();                   
                    tlStrStat_ServerName.Text = serviceName;
                    tlStrStat_Activity.Text = languages.statusRunning(language);
                }
            }
        }


        private void initiateTimer()
        {
            timer1.Tick += new EventHandler(timer1_Tick);
            timer1.Interval = 1800;
            timer1.Start();
        }
        private void timer1_Tick(object sender, EventArgs e)
        {
            timer1.Stop();
            tlStrStat_ServerName.Text = "";
            tlStrStat_Activity.Text = "";
        }

        private void btn_Stop_Click(object sender, EventArgs e)
        {
            foreach (ServiceController sc in sl.SQLsList())
            {
                string name = sc.ServiceName.ToString();
                if (name.Equals(serviceName))
                {
                    sc.Stop();
                    sc.WaitForStatus(ServiceControllerStatus.Stopped);
                    serviceStatus(serviceName);
                    initiateTimer();
                    tlStrStat_ServerName.Text = serviceName;
                    tlStrStat_Activity.Text = languages.statusStopped(language);
                }
            }
        }

        private void btn_Restart_Click(object sender, EventArgs e)
        {
            foreach (ServiceController sc in sl.SQLsList())
            {
                string name = sc.ServiceName.ToString();
                if (name.Equals(serviceName))
                {
                    sc.Stop();
                    sc.WaitForStatus(ServiceControllerStatus.Stopped);
                    sc.Start();
                    sc.WaitForStatus(ServiceControllerStatus.Running);
                    serviceStatus(serviceName);
                    initiateTimer();
                    tlStrStat_ServerName.Text = serviceName;
                    tlStrStat_Activity.Text = languages.statusRestarted(language);
                }
            }
        }

        private void filerToolStripMenuItem_Click(object sender, EventArgs e)
        {

        }

        private void afslutToolStripMenuItem_Click(object sender, EventArgs e)
        {
            this.Dispose();
        }

        private void englishToolStripMenuItem_Click(object sender, EventArgs e)
        {
            language = "english";
            initMenuItems();
            initComponents();
        }

        private void danishToolStripMenuItem_Click(object sender, EventArgs e)
        {
            language = "dansk";
            initMenuItems();
            initComponents();
        }

        private void aboutToolStripMenuItem_Click(object sender, EventArgs e)
        {
            AboutBox1 a = new AboutBox1();
            a.Show();
        }

        private void Form1_Resize(object sender, System.EventArgs e)
        {
            if (FormWindowState.Minimized == WindowState)
                Hide();
        }

        private void notifyIcon1_DoubleClick(object sender,
                                    System.EventArgs e)
        {
            Show();
            WindowState = FormWindowState.Normal;
        }

        private void toolStripMenuItem1_Click(object sender, EventArgs e)
        {
            Show();
            WindowState = FormWindowState.Normal;
        }

        private void closeSQLCommanderToolStripMenuItem_Click(object sender, EventArgs e)
        {
            this.Dispose();
        }

        private void startMenuNotifyContextMenu()
        {
            foreach (ServiceController str in sl.SQLsList())
            {
                string scName = str.ServiceName.ToString();
                ToolStripMenuItem menuItm = new ToolStripMenuItem(scName, null,  new EventHandler(btn_Start_Click));
                menuItm.Name = scName;
                startToolStripMenuItem.DropDownItems.Add(menuItm);
            }
        }
    }
}


//hellfishdk
Avatar billede arne_v Ekspert
05. juni 2010 - 20:48 #1
6 MB er ingen ting idag.

.NET er beregnet til moderne maskiner ikke maskiner fra 1995.

.NET runtime og de brugte framework klasser fylder en del.
Avatar billede hellfishdk Nybegynder
05. juni 2010 - 20:59 #2
hehe ok :)

Så kan jeg være mere rolig omkring størrelse.

Hvad med koden? Her tænker jeg særligt på SQLsList. Er der noget som kan gøres bedre?
Avatar billede arne_v Ekspert
05. juni 2010 - 21:16 #3
1) du behøver ikke kopiere fra en List til en anden List med en løkke da List har en AddRange metode som tager en hel liste
Avatar billede arne_v Ekspert
05. juni 2010 - 21:18 #4
2)

somelist.Clear();
somelist = null;

er et eller andet sted mellem totalt overfødigt og direkte skadeligt.

Bare lad den gå ud af scope of lad garbage collectoren håndtere resterne.
Avatar billede arne_v Ekspert
05. juni 2010 - 21:19 #5
3)

i C# kan du godt sammenligne strenge med ==
Avatar billede arne_v Ekspert
05. juni 2010 - 21:21 #6
4)

setLanguage

set lidt halvfærdig ud
Avatar billede hellfishdk Nybegynder
05. juni 2010 - 23:50 #7
setLanguage og getLanguage, er leftovers fra et problem jeg løste på en anden måde i stedet. Så de skal bare slettes.

#4 - hvordan kan det være skadeligt at clear en liste, og sætte den til null?

#5 - er det et bestemt sted du tænker på jeg skal bruge ==?
Avatar billede arne_v Ekspert
06. juni 2010 - 00:00 #8
Jo længere tid du bliver ved med at bruge referance jo længere tid inden garbage collectoren frigiver hukommelsen.

Clear laver sikker også en del totalt oveflødig når objektet ikke skal bruges mere.
Avatar billede arne_v Ekspert
06. juni 2010 - 00:01 #9
if (name.Equals(serviceName))

ligner en sammenligning af 2 strings
Avatar billede janus_007 Nybegynder
06. juni 2010 - 23:09 #10
Hej hellfishdk

Jeg ser flere ting som kunne gøres bedre :)

Husk at uppercase(naming convention fra Microsoft) dine string sammenligninger eks.vis status.Equals("Running") => status.ToUpper() == "RUNNING" , brug ikke equals, den er tungere.

Husk også at skrive metodenavne i PascalCasing (Microsoft convention)

Du henter ServiceControllers og differencierer på hver ud med forskelligt navn men samme type. Det er meget oldschool .NET 2.0, brug hellere Linq og lad være med at spilde kostbar tid på at iterere igennem.
Når du bruger Linq så har du ServiceName lige ved hånden, dvs. istedet for at gøre sådan her(gælder alle 3), dvs. hele din SQLsList

//Get ORACLEXE servers
            foreach (ServiceController scTemp in services)
            {
                //get MSSQL servers
                if (scTemp.ServiceName.Contains("OracleService"))
                {
                    oracleList.Add(scTemp);
                }
            }

Kan du udskifte med en linje :-O , altså:

return services.Where(x => x.ServiceName == "oracleservice") -- not case sensitiv



Og så her hvor du bruger den :
foreach (ServiceController str in sl.SQLsList())
            {
                string scName = str.ServiceName.ToString();
                if (scName.Length > ln)
                    ln = scName.Length;
                bList.Add(scName);
            }
            comboBox1.Width = ln*9;
            comboBox1.DataSource = bList;


Ja den kan du også udskifte med et par linjer:

var service = sl.SQLsList();
comboBox1.Width = service.Max(x => x.Length) * 9
comboBox1.DataSource = service.Select(x => x.ServiceName.ToString());


Som du kan se er din kode nu allerede gået fra godt 75-80 linjer til 3 linjer.
Dine forskellige init-sager kan dårligt laves anderledes, så tænk ikke på det endnu. Men det du kan gøre er at erstatte dine øvrige .Net 2.0 med .Net 3.5 som jeg har giver eksempler på.

Derudover som arne siger, så glem dine Clear og nul, det nytter nada :)

God fornøjelse.
Avatar billede arne_v Ekspert
07. juni 2010 - 02:58 #11
Equals er ikke tungere end ==.

== kalder faktisk Equals.

Grunden til at man bør bruge == er ikke performance men readability.
Avatar billede arne_v Ekspert
07. juni 2010 - 03:05 #12
foreach (ServiceController scTemp in services)
            {
                //get MSSQL servers
                if (scTemp.ServiceName.Contains("OracleService"))
                {
                    oracleList.Add(scTemp);
                }
            }


kan ikke udskiftes med:

return services.Where(x => x.ServiceName == "oracleservice") -- not case sensitiv

fordi:
* == i LINQ to objects *er* case sensitiv
* ServiceName vil have værdier "OracleServiceXxxx"

return services.Where(x => x.ServiceName.Contains("OracleService"))

vil sikkert virke
Avatar billede janus_007 Nybegynder
07. juni 2010 - 12:46 #13
Der skulle nu også stå case sensitiv, jeg havde jo lige skrevet om uppercase mv.

Jeg var ikke klar over værdien, men det er jo såre simpelt. Enten med en Contains, men en StartsWith er nu at foretrække.

Altså:
return services.Where(x => x.ServiceName.StartsWith("OracleService"))

NB: Ja du har ret med Equals, læste lige op på det.

Men summasummarum, vi er stadig gået fra 75-80 linjer til 3 linjer. Dejlig optimering :)
Avatar billede hellfishdk Nybegynder
08. juni 2010 - 00:17 #14
Nu kender jeg ikke noget til linq. Men det er da vist noget jeg skal se lidt nærmere på :)

Smider i et svar begge to? Jeg tænker 60 pts til janus_007, pga. det med linq, og 40 til arne_v med de indledende svar.
Avatar billede arne_v Ekspert
08. juni 2010 - 02:46 #15
svar
Avatar billede janus_007 Nybegynder
08. juni 2010 - 10:33 #16
Du spørger bare vedr. Linq, det er desværre ikke så udbredt endnu, så hvis jeg kan hjælpe med til at flytte folk fra 2.0 til 3.5+ er jeg glad.
Avatar billede janus_007 Nybegynder
08. juni 2010 - 10:34 #17
.
Avatar billede Ny bruger Nybegynder

Din løsning...

Tilladte BB-code-tags: [b]fed[/b] [i]kursiv[/i] [u]understreget[/u] Web- og emailadresser omdannes automatisk til links. Der sættes "nofollow" på alle links.

Loading billede Opret Preview
Kategori
IT-kurser om Microsoft 365, sikkerhed, personlig vækst, udvikling, digital markedsføring, grafisk design, SAP og forretningsanalyse.

Log ind eller opret profil

Hov!

For at kunne deltage på Computerworld Eksperten skal du være logget ind.

Det er heldigvis nemt at oprette en bruger: Det tager to minutter og du kan vælge at bruge enten e-mail, Facebook eller Google som login.

Du kan også logge ind via nedenstående tjenester