[c#] struct/arraylist, waardes overschreven?

[TeeDee]

Voor een tooltje wat ik voor mezelf aan het bouwen ben heb ik de volgende struct:

C#:

public struct DiskInfoStruct
{
    public string Name,FileSystem,FreeSpace,Size;
    public DiskInfoStruct(string name, string filesystem, string freespace,string size)
    {
        Name = name;
        FileSystem = filesystem;
        FreeSpace=freespace;
        Size=size;
    }
}

Vervolgens heb ik een method welke het aantal logical disks opzoekt. (In dit geval 2)
Tevens heb ik ook de volgende ArrayList

C#:

public ArrayList DiskInformation()
{
    ArrayList ht = new ArrayList();
    ArrayList myArr = obj.ObjectPropertyArray("Win32_LogicalDisk where DriveType=3");
    DiskInfoStruct distruct = new DiskInfoStruct();
    for(int c=0;c<_numberOfHardDisks;c++)
    {
        for(int i=0;i<myArr.Count;i++)
        {
            PropertyData prop = (PropertyData)myArr[i];
            if(prop.Name=="Name") distruct.Name=prop.Value.ToString();
            if(prop.Name=="FreeSpace")  distruct.FreeSpace=prop.Value.ToString();
            if(prop.Name=="Size")   distruct.Size=prop.Value.ToString();
            if(prop.Name=="FileSystem") distruct.FileSystem=prop.Value.ToString();
        }
        ht.Add(distruct);
    }
    return ht;  
}

Vervolgens wil ik dit in de GUI uitlezen mbv:

C#:

SystemStatsLibrary.DiskInfo disk = new DiskInfo();
ArrayList ht = disk.DiskInformation();
for(int i = 0;i<ht.Count;i++)
{
    DiskInfoStruct di = (DiskInfoStruct)ht[i];
    lblInfo.Text+=di.Name;
}

Ik heb 2 harddisks in mijn systeem heb zitten. C: en E: Als ik het appje run, zie ik 2x E: staan ipv C: en E:.

Als ik de complete collectie van DiskInfoArray doorloop, dan zie ik netjes de 2 verschillende schijven staan. Ergens wordt er dus de waarde voor de C: schijf overschreven. Komt dit door de struct of doe ik iets compleet fout in het wegschrijven van de waardes in de ArrayList DiskInformation()?

Edit: het betreft hier een 1.1 project, dus Generics oid (weet nog niet zeker of die hier handig bij zijn) kan ik niet gebruiken.

[ Voor 3% gewijzigd door TeeDee op 13-03-2007 23:34 ]

[RobIII]

Is het niet handiger als je "DiskInfoStruct distruct = new DiskInfoStruct();" in je for lus zet, zodat je ook daadwerkelijk nieuwe instances toevoegt aan je arraylist i.p.v. 2 keer dezelfde?

[ Voor 6% gewijzigd door RobIII op 14-03-2007 01:15 ]

[Not Pingu]

RobIII schreef op woensdag 14 maart 2007 @ 01:15:
Is het niet handiger als je "DiskInfoStruct distruct = new DiskInfoStruct();" in je for lus zet, zodat je ook daadwerkelijk nieuwe instances toevoegt aan je arraylist i.p.v. 2 keer dezelfde?

Zou bij een struct goed moeten gaan toch? Da's een value type.

[RobIII]

Not Pingu schreef op woensdag 14 maart 2007 @ 01:24:
[...]


Zou bij een struct goed moeten gaan toch? Da's een value type.

Correct, my bad...
_{je ondertitel is nu wel erg pijnlijk}

[ Voor 11% gewijzigd door RobIII op 14-03-2007 01:49 ]

[whoami]

Ik snap niet wat het doel is van die buitenste lus in je eerste code-voorbeeld ?
Je gebruikt nergens die variable c ?

[TeeDee]

whoami schreef op woensdag 14 maart 2007 @ 08:54:
Ik snap niet wat het doel is van die buitenste lus in je eerste code-voorbeeld ?
Je gebruikt nergens die variable c ?

Mjah, die zou in eerste instantie moeten dienen als een index voor een Hashtable, omdat me dat in eerste instantie beter leek dan een ArrayList.

Misschien zit ik met een struct wel te moeilijk te doen.

Heb het nu wel imo netjes aan de praat:

C#:

public Hashtable DiskInformation()
{
    Hashtable disks = new Hashtable();
    ManagementObjectSearcher DiskSearch = 
        new ManagementObjectSearcher("Select * from Win32_LogicalDisk where DriveType=3");
    ManagementObjectCollection DiskCollection = DiskSearch.Get();
    int enumu=0;
    foreach(ManagementObject mo in DiskCollection)
    {
        DiskInfoStruct dskinfo = new DiskInfoStruct();
        dskinfo.Name = mo["Name"].ToString();
        dskinfo.FileSystem = mo["FileSystem"].ToString();
        dskinfo.Size= mo["Size"].ToString();
        dskinfo.FreeSpace= mo["FreeSpace"].ToString();
        disks.Add(enumu,dskinfo);
        mo.Dispose();
        enumu++;
    }
    return disks;
}

Met dezelfde struct als in de TopicStart. Dit lees ik uit in de GUI met

C#:

Hashtable ht = disk.DiskInformation();
for(int i=0;i<numDisks;i++)
{
    DiskInfoStruct di = new DiskInfoStruct();
    di = (DiskInfoStruct)ht[i];
    lblInfo.Text+=di.Name+Environment.NewLine;
    lblInfo.Text+=di.FileSystem+Environment.NewLine;
    lblInfo.Text+=di.FreeSpace+Environment.NewLine;
    lblInfo.Text+=di.Size+Environment.NewLine;
    lblInfo.Text+="----------------------"+Environment.NewLine;
}

(de interface komt er anders uit te zien, dit is alleen even om de Library te testen )
Ik heb eigenlijk geen gegronde reden voor het gebruik van een Hashtable, vind ze nu eenmaal fijner werken.
Anyway, ik heb het nu werkend. Alleen de vraag is: Zouden jullie dit anders doen? Dus het hele struct verhaal overslaan ofzo.

[ Voor 64% gewijzigd door TeeDee op 14-03-2007 10:53 ]

[DrDelete]

Ik zou public methods altijd typed teruggeven en niet in de vorm van Hashtable of Arraylist omdat je voor het ophalen telkens moet casten wat weer performanceverlies geeft en kans op fouten vergroot. Je kunt Hashtable of ArrayList wel prima gebruiken voor private methods aangezien dit met encapsulatie afgeschermd wordt van de buitenwereld.

[TeeDee]

Dit idee bedoel je?

C#:

public DiskInfo DiskInformation()
{
    //
}

Hoe zie je dat voor je met de genoemde struct? Deze zal een soort van Array van DiskInfoStructs gaan bevatten.

[Not Pingu]

Ik denk dat ie bedoelt dat je beter een array van DiskInfo kunt returnen. Gewoon zo:

C#:

public DiskInfo[] DiskInformation()
{
    ArrayList list = new ArrayList();
    //list vullen
    return (DiskInfo[])list.ToArray();
}

Of zelf een DiskInfo[] aanmaken en vullen voor wat meer performance: geen cast nodig, geen ArrayList die intern nog aardig wat werk verzet.

[ Voor 10% gewijzigd door Not Pingu op 14-03-2007 12:21 ]

[TeeDee]

Volgens mij moet ik me nog eens in een aantal zaken gaan verdiepen.

C#:

    public class DiskInfo
    {
        public DiskInfo()
        {}
        public DiskInfo[] DiskInformation()
        {
            ArrayList disks = new ArrayList();
            //ManagementObject etc.
            foreach(ManagementObject mo in DiskCollection)
            {
                DiskInfoStruct dskinfo= new DiskInfoStruct();
                //vul de DiskInfoStruct
                disks.Add(dskinfo);
                mo.Dispose();
            }
            return (DiskInfo[])disks.ToArray();
        }
    }
}

Geeft een prachtige "Specified Cast is not valid"

[Woy]

Not Pingu schreef op woensdag 14 maart 2007 @ 12:20:
Ik denk dat ie bedoelt dat je beter een array van DiskInfo kunt returnen. Gewoon zo:

C#:

public DiskInfo[] DiskInformation()
{
    ArrayList list = new ArrayList();
    //list vullen
    return (DiskInfo[])list.ToArray();
}

Of zelf een DiskInfo[] aanmaken en vullen voor wat meer performance: geen cast nodig, geen ArrayList die intern nog aardig wat werk verzet.

Hoezo dan niet gewoon
[code=c#]
public List<DiskInfo> DiskInformation()
{
List<DiskInfo> list = new List<DiskInfo>();
//list vullen
return list;
}
[/code]
Als het .net 2.0 is ieder geval.
Niet goed gelezen. .NET 1.1 dus

[Verwijderd]

DrDelete schreef op woensdag 14 maart 2007 @ 11:59:
...omdat je voor het ophalen telkens moet casten wat weer performanceverlies geeft...

Is Casten zo'n domper op performance dan?

[Woy]

Verwijderd schreef op woensdag 14 maart 2007 @ 12:56:
[...]
Is Casten zo'n domper op performance dan?

Nee dat valt wel mee. Het vergroot echter wel de kans op fouten, als het mogenlijk is kan je beter casten zo veel mogenlijk vermijden.

[Not Pingu]

TeeDee schreef op woensdag 14 maart 2007 @ 12:51:
Volgens mij moet ik me nog eens in een aantal zaken gaan verdiepen.
[..]
Geeft een prachtige "Specified Cast is not valid"

Oeps, je struct heet DiskInfoStruct en niet DiskInfo. My bad, maar eigenlijk is dat een slechte typebenaming

hm, ik lees nu trouwens dit:

The CLR does not allow the casting of arrays with value type elements to any other type. However, using the Array.Copy method, you can create a new array that has the desired effect

Dit werkt wel:

C#:

ArrayList list = new ArrayList();
//list vullen
DiskInfoStruct[] diskinfostructs = new DiskInfoStruct[list.Count];
list.CopyTo(diskinfostructs);
return diskinfostructs;

rwb schreef op woensdag 14 maart 2007 @ 13:25:
[...]

Nee dat valt wel mee. Het vergroot echter wel de kans op fouten, als het mogenlijk is kan je beter casten zo veel mogenlijk vermijden.

Het ligt er ook aan hoe vaak het gebruikt wordt. Stel bijv. je doet iets met realtime graphics waarmee je per frame bijv. 800x600x16 lookups moet doen in een framebuffer (antialiasing ofzo, ik noem maar iets), dan ga je het verschil tussen wel of geen cast in je lookupfunctie al snel merken.

[ Voor 59% gewijzigd door Not Pingu op 14-03-2007 13:48 ]

[TeeDee]

edit, even alle rommel weg.
Volgens mij heb ik het nu zoals ik wil.

C#:

public class DiskInfo
{
    public DiskInfo()
    {}
    public DiskInfoStruct[] Test()
    {
        DiskInfoStruct[] di=new DiskInfoStruct[2];
        di[0]=new DiskInfoStruct("Naam1","NTFS","10000000","20000000");
        di[1]=new DiskInfoStruct("Naam2","FAT","10000000","20000000");
        return(di);
    }
}

De code van de struct:

C#:

public struct DiskInfoStruct
{
    public string Name,FileSystem,FreeSpace,Size;
    public DiskInfoStruct(string name, string filesystem, string freespace,string size)
    {
        Name = name;
        FileSystem = filesystem;
        FreeSpace=freespace;
        Size=size;
    }
}

En deze wordt in de GUI aangeroepen met:

C#:

DiskInfoStruct[] diskArr=disk.Test();
Debug.WriteLine(diskArr.Length);

Hoppa.

Not Pingu schreef op woensdag 14 maart 2007 @ 13:31:
[...]


Oeps, je struct heet DiskInfoStruct en niet DiskInfo. My bad, maar eigenlijk is dat een slechte typebenaming

Waarom is DiskInfoStruct niet goed als ik vragen mag?

Dit werkt wel:

C#:

ArrayList list = new ArrayList();
//list vullen
DiskInfoStruct[] diskinfostructs = new DiskInfoStruct[list.Count];
list.CopyTo(diskinfostructs);
return diskinfostructs;

Dat probeer ik ook nog even.

[ Voor 55% gewijzigd door TeeDee op 14-03-2007 14:02 ]

[Not Pingu]

TeeDee schreef op woensdag 14 maart 2007 @ 13:45:
Waarom is DiskInfoStruct niet goed als ik vragen mag?

offtopic:
Tis maar imho, maar het Struct erachteraan is redundant omdat je al in de intellisense kunt zien dat het een struct is. Aangezien je nog aan het leren bent, is het wel begrijpelijk.

[TeeDee]

Not Pingu schreef op woensdag 14 maart 2007 @ 13:55:
[...]

offtopic:
Tis maar imho, maar het Struct erachteraan is redundant omdat je al in de intellisense kunt zien dat het een struct is. Aangezien je nog aan het leren bent, is het wel begrijpelijk.

Duidelijk. Ik moet ook nog alle blaat, melp, meuk e.d. eruit vissen

[DrDelete]

rwb schreef op woensdag 14 maart 2007 @ 13:25:
[...]

Nee dat valt wel mee. Het vergroot echter wel de kans op fouten, als het mogenlijk is kan je beter casten zo veel mogenlijk vermijden.

Door casting te doen op structs krijg je te maken met boxing en unboxing wat performanceverlies oplevert. Je kunt beter de entiteiten als classes definieren waardoor je geen (un)boxing hebt.

Ik kan je het volgende boek zeer aanraden: Effective C# door Bill Wagner. Verder zijn de design guidelines for developers van MS ook van belang inzake collections.

[ Voor 15% gewijzigd door DrDelete op 14-03-2007 14:21 ]

[DrDelete]

Not Pingu schreef op woensdag 14 maart 2007 @ 12:20:
Ik denk dat ie bedoelt dat je beter een array van DiskInfo kunt returnen. Gewoon zo:

C#:

public DiskInfo[] DiskInformation()
{
    ArrayList list = new ArrayList();
    //list vullen
    return (DiskInfo[])list.ToArray();
}

Of zelf een DiskInfo[] aanmaken en vullen voor wat meer performance: geen cast nodig, geen ArrayList die intern nog aardig wat werk verzet.

Ik bedoel: retourneer een collectie van DiskInfo

dus:

code:

  public class DiskInfo
  {
      fields + properties
  }

  public class DiskInfoCollection : CollectionBase
  {
  }

[Woy]

DrDelete schreef op woensdag 14 maart 2007 @ 14:19:
[...]


Door casting te doen op structs krijg je te maken met boxing en unboxing wat performanceverlies oplevert. Je kunt beter de entiteiten als classes definieren waardoor je geen (un)boxing hebt.

Ik kan je het volgende boek zeer aanraden: Effective C# door Bill Wagner. Verder zijn de design guidelines for developers van MS ook van belang inzake collections.

Tuurlijk levert het boxen/unboxen performace verlies op. Maar naar een andere collection kopieren levert ook performance verlies op. Een nieuw type collection introduceren zoals je aangeeft zou wel een oplossing zijn, maar als je dat voor alle types moet doen is dat ook een hoop meerwerk. Met generics is dat een stuk makkelijker geworden en kan je gewoon de Generic List pakken.

Wat ik probeer aan te geven is dat je je niet altijd blind moet staren op performance als het eigenlijk helemaal geen issue is. Als het iets is wat voor elke pixel in een frame gedaan moet worden is het natuurlijk wel weer de moeite om je er mee bezig te houden.

[DrDelete]

rwb schreef op woensdag 14 maart 2007 @ 14:29:
[...]

Tuurlijk levert het boxen/unboxen performace verlies op. Maar naar een andere collection kopieren levert ook performance verlies op. Een nieuw type collection introduceren zoals je aangeeft zou wel een oplossing zijn, maar als je dat voor alle types moet doen is dat ook een hoop meerwerk. Met generics is dat een stuk makkelijker geworden en kan je gewoon de Generic List pakken.

Wat ik probeer aan te geven is dat je je niet altijd blind moet staren op performance als het eigenlijk helemaal geen issue is. Als het iets is wat voor elke pixel in een frame gedaan moet worden is het natuurlijk wel weer de moeite om je er mee bezig te houden.

De topicstart gaf aan dat ie met 1.1 werkte, vandaar dat ik de optie met CollectionBase aanhaalde.

Getypeerd werken met collections levert ook beter onderhoudbare code op, het is niet alleen maar performance.

[TeeDee]

C#:

public class DiskInfoCollection : CollectionBase
{}

Dat ga ik nog even proberen. Ik was daar niet opgekomen en gevoelsmatig lijkt me dit de beste oplossing.

Voor de duidelijkheid: Ik heb het nu wel werkend, maar door een DiskInfo[] te retourneren.

[Woy]

DrDelete schreef op woensdag 14 maart 2007 @ 14:38:
[...]

De topicstart gaf aan dat ie met 1.1 werkte, vandaar dat ik de optie met CollectionBase aanhaalde.

Getypeerd werken met collections levert ook beter onderhoudbare code op, het is niet alleen maar performance.

Tuurlijk ben ik met je eens dat dat de netste oplossing is. Maar het is een beetje overkill om voor elke situatie een nieuwe gespecialiseerde collection te introduceren. Dat zal je per geval af moeten wegen.

[whoami]

TeeDee schreef op woensdag 14 maart 2007 @ 12:51:
Volgens mij moet ik me nog eens in een aantal zaken gaan verdiepen.

C#:

    public class DiskInfo
    {
        public DiskInfo()
        {}
        public DiskInfo[] DiskInformation()
        {
            ArrayList disks = new ArrayList();
            //ManagementObject etc.
            foreach(ManagementObject mo in DiskCollection)
            {
                DiskInfoStruct dskinfo= new DiskInfoStruct();
                //vul de DiskInfoStruct
                disks.Add(dskinfo);
                mo.Dispose();
            }
            return (DiskInfo[])disks.ToArray();
        }
    }
}

Geeft een prachtige "Specified Cast is not valid"

Een arraylist casten naar een array van bepaalde objecten doe je zo:

code:

return (DiskInfo[])disks.ToArray(typeof(DiskInfo));

[TeeDee]

Hmm, ik leer elke dag bij zie ik. Bedankt weer voor alle handige tips mensen.