Kirjautuminen

Haku

Tehtävät

Keskustelu: Ohjelmointikysymykset: Java: Koodin siistiminen

johan123 [28.06.2006 13:15:02]

#

Voisko joku siisti vähän tätä aloittelijan (minun) koodia oppimismielessä että tietäisi miten homman sais paremmin toimimaan.

Kyseessä on siis kolme funktioota jotka täyttävät "puu-kontrollin" kahdella eri objektilla, asiakas ja asiakas ryhmä. Jos koodi on liian epäselvä voin koittaa vähän kommentoida sitä mutta en oiken tiedä että mitä pitäisi kommentoida...

(private customTreeNode node luokka deklaraatio)
private void createTree()
        {
            treeCustomers.Nodes.Clear();
            if (customerGroups == null)
                return;
            else
            {
                foreach (customerGroup group in customerGroups)
                {
                    if ((group.getGroupParentID() == 0) || (group.getLevel() == 1))
                    {
                        node = null;
                        node = new customNodeObject(group);

                        ArrayList list;
                        list = findClient(group.getGroupName());
                        if (list != null)
                        {
                            foreach (customNodeObject c in list)
                            {
                                customNodeObject newerNode = c;
                                newerNode.ForeColor = Color.Green;
                                node.Nodes.Add(newerNode);
                            }
                        }

                        loopNodes(customerGroups, (group.getLevel() + 1), group, node);
                        treeCustomers.Nodes.Add(node);
                    }
                }
            }
            treeExpander();
        }

        private System.Collections.ArrayList findClient(String type)
        {
            ArrayList custTemp = new ArrayList();
            foreach (customer cust in customers)
            {
                if ((type == cust.CustType) && (cust.UpdateStatus != "delete"))
                {
                    custTemp.Add(new customNodeObject(cust));
                }
            }
            return custTemp;
        }

        private void loopNodes(ArrayList customerGroups, int previousLevel, customerGroup parent, customNodeObject node)
        {
            foreach (customerGroup group in customerGroups)
            {
                if ((group.getLevel() == previousLevel) && (parent.getID().Equals(group.getGroupParentID())))
                {
                    int nextLevel = (previousLevel + 1);
                    customNodeObject newNode = new customNodeObject(group);

                    ArrayList list;
                    list = findClient(group.getGroupName());
                    if (list != null)
                    {
                        foreach (customNodeObject c in list)
                        {
                            customNodeObject newerNode = c;
                            newerNode.ForeColor = Color.Green;
                            newNode.Nodes.Add(newerNode);
                        }
                    }
                    node.Nodes.Add(newNode);
                    loopNodes(customerGroups, nextLevel, group, newNode);
                }
            }
        }

anttipanda [29.06.2006 12:47:10]

#

Jos käytät .NET 2.0:aa, niin kannattaa aluksi hankkiutua eroon noista ArrayListeistä. List<> on mukavampi tapa käsitellä tietoja. Kannattaa myös nimetä kaikki luokat ja metodit isolla alkukirjaimella .NET-konvention mukaisesti. Nuo getterit kannattaa tehdä propertyiksi.

findClient -metodissa ei ehkä kannata palauttaa ArrayListiä, vaan taulukko riittää. Eikä se ikinä palauta nullia, joten listan tarkistus on ihan turha. Tässä muutaman muutoksen kokenut koodi (vanhat rivit jätetty kommentteina):

#edit: muotoilut meni ihan päin prinkkalaa, mutta Visual Studiossa ainakin kun painat CTRL pohjaan + A+K+F niin se muotoilee automaattisesti. En ole luonnollisesti voinut testata yhtään, eikä tämä toimi suoraan sinun koodin kanssa. Ideana olisi se, että vaihdat ensin alla mainittujen tunnisteiden nimet ja tyypit, niin sitten pitäisi alkaa toimimaan. Jos käytät Visual Studio 2005:ttä tai Express-versiota, niin nimien vaihtaminen käy kätevästi refaktorointiominaisuuksien avulla.

//tässä on muutama uudelleennimeäminen
//customerGroup -> CustomerGroup
//  getGroupParentID() -> ParentId, tee property
//  getLevel() -> Level, tee property
//  getGroupName() -> Name, tee property
//  getID() -> Id, tee property
//customNodeObject -> CustomNodeObject
//treeExpander() -> ExpandTree()
//customer -> Customer
//  CustType -> Type

//private void createTree()
private void CreateTree()
{
    treeCustomers.Nodes.Clear();
    if (customerGroups == null)
    {
        return;
    }
    else
    {
        //foreach (customerGroup group in customerGroups)
        foreach(CustomerGroup group in customerGroups)
        {
            //if ((group.getGroupParentID() == 0) || (group.getLevel() == 1))
            if( group.ParentId == 0 || group.Level == 1)
            {
                node = null;
                //node = new customNodeObject(group);
                node = new CustomNodeObject(group);

                //ArrayList list;
                //list = findClient(group.getGroupName());
                CustomNodeObject[] customNodes = FindClient(group.Name);
                //if (list != null) //turha
                //{
                    //foreach (customNodeObject c in list)
                    foreach( CustomNodeObject customNode in customNodes)
                    {
                        //customNodeObject newerNode = c; //turha
                        //newerNode.ForeColor = Color.Green;
                        //node.Nodes.Add(newerNode);

                        customNode.ForeColor = Color.Green;
                        node.Nodes.Add(customNode);
                    }
                //}

                //LoopNodes(customerGroups, (group.getLevel() + 1), group, node);
                LoopNodes(customerGroups.ToArray(), (group.Level + 1), group, node);
                treeCustomers.Nodes.Add(node);
            }
        }
    }
    //treeExpander();
    ExpandTree();
}

//private System.Collections.ArrayList findClient(String type)
private CustomNodeObject[] FindClient(string type)
{
    //ArrayList custTemp = new ArrayList();
    List<CustomNodeObject> customersTemp = new List<CustomNodeObject>();
    //foreach (customer cust in customers)
    foreach (Customer customer in customers)
    {
        //if ((type == cust.CustType) && (cust.UpdateStatus != "delete"))
        if ((type == customer.Type) && (customer.UpdateStatus != "delete"))
        {
            //custTemp.Add(new customNodeObject(customer));
            customersTemp.Add(new CustomNodeObject(customer));
        }
    }
    //return custTemp;
    return customersTemp.ToArray();
}

//private void loopNodes(ArrayList customerGroups, int previousLevel, customerGroup parent, customNodeObject node)
private void LoopNodes(CustomerGroup[] customerGroups, int previousLevel, CustomerGroup parent, customNodeObject node)
{
    //foreach (customerGroup group in customerGroups)
    foreach (CustomerGroup group in customerGroups)
    {
        //if ((group.getLevel() == previousLevel) && (parent.getID().Equals(group.getGroupParentID())))
        if ((group.Level == previousLevel) && (parent.Id == group.ParentId))
        {
            int nextLevel = (previousLevel + 1);
            //customNodeObject newNode = new customNodeObject(group);
            CustomNodeObject newNode = new CustomNodeObject(group);

            //ArrayList list;
            //list = findClient(group.getGroupName());
            CustomNodeObject[] customNodes = FindClient(group.Name);
            //if (list != null) //turha
            //{
                //foreach (customNodeObject c in list)
                foreach (CustomNodeObject customNode in customNodes)
                {
                    //customNodeObject newerNode = c;
                    //newerNode.ForeColor = Color.Green;
                    //newNode.Nodes.Add(newerNode);
                    customNode.ForeColor = Color.Green;
                    newNode.Nodes.Add(customNode);
                }
            //}
            node.Nodes.Add(newNode);
            LoopNodes(customerGroups, nextLevel, group, newNode);
        }
    }
}

(Mod. edit: kooditagit ovat suotavat.)

johan123 [29.06.2006 14:47:20]

#

Jes, kiitti, tulipa taas opittua jotain uutta. Tuo mun nimeäminen ja getterit ja setterit ovat Javasta peräisin. Jotenkin nuo .net:in propertyt tuntuvat epäluonollisilta, mutta sekin on vain tottuma kysymys. Taidankin vaihtaa samantien kaikki getterit propertyks, säästyy myös mukavasti aikaa kun VS 2005 näköjään tekee sen automaattisesti Encapsulate Field komennolla...

Kiitos

Vastaus

Aihe on jo aika vanha, joten et voi enää vastata siihen.

Tietoa sivustosta