Avatar billede tdh1309 Juniormester
18. august 2009 - 21:22 Der er 30 kommentarer og
1 løsning

Kode fejler

Hej

Er meget ny i C# og objektorienteret udvikling generelt!
Kan nogle hjælpe mig med hvad der går galt?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Yatzy
{
    // DiceCup class
    // Represents DiceCup containing five dices
    // Methods:
    // Throw: Throws dices (whics are not hold)
    // GetDice():          Returns value of dice X
    // SetHold (DiceNo):  SetsHold on dice number X
    // SetHoldToAll ():    Sets all dices on hold
    // UnsetHold (DiceNo): UnHold dice number X
    // UnsetHoldAll ():    No dices are on hold
    // Properties:
    // arrDices: Two dimensional array containg dice(s) and
    //          hold value for each dice (0 = hold, 1 = throw)
    //          Default is hold value set to 1 (throw)

    class DiceCup
    {
        public Dice[] arrDices;
        public int NumberOfDices;

        // Constructor: create x dices       
        public DiceCup(int NumberOfDicesAA)
        {
            NumberOfDices = NumberOfDicesAA;
            Console.Write("NumberOfDices {0}",NumberOfDices);
           
            arrDices = new Dice[NumberOfDices];
            Console.Write("arrDices {0}", arrDices.Length);
        }

        // Throws dices in cup
        public void ThrowDices()
        {
            for (int ArrLoop = 0; ArrLoop < NumberOfDices; ArrLoop++)
            {
                //arrDices[ArrLoop].SetHoldOnDic;
                arrDices[ArrLoop].ThrowDice();
            }
        }

        // Returns values of each dice in aray, starting from Dice1
        public int[] GetDices()
        {
            int[] DicesInCup = new int[NumberOfDices];
            for (int ArrLoop = 1; ArrLoop < NumberOfDices; ArrLoop++)
            //foreach (ArrLoop in arrDices)
            {
                Console.Write("ArrLoop {0}", ArrLoop);
                //DicesInCup[ArrLoop] = arrDices[ArrLoop].GetDice();
                Console.Write(arrDices[ArrLoop].GetDice());
                Console.ReadLine();
            }
            return DicesInCup;
        }

        // Hold all dices
        public void SetHoldToAll()
        {
            for (int ArrLoop = 0; ArrLoop < NumberOfDices; ArrLoop++)
            {
                //arrDices[ArrLoop].SetHoldOnDic;
                arrDices[ArrLoop].SetHoldOnDice();
            }
        }

        // UnsetHold on all dices
        public void UnSetHoldToAll()
        {
            for (int ArrLoop = 0; ArrLoop < NumberOfDices; ArrLoop++)
            {
                arrDices[ArrLoop].UnSetHoldOnDice();
            }
        }
    }
}


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Yatzy
{
    // Dice class
    // Represents one Dice
    // Methods:
    //  ThrowDice: Assign random value between 1 - 6 to dice
    //  GetDice:  Returns value of current dice
    // Properties:
    //  DiceValue Value af Dice
    //  HoldDice  True - dice cannot by thorn
    class Dice
    {
        private int DiceValue = 1;      // Value of dice
        private bool HoldDice = false;  // Set if dice can be thrown

        public Dice()
        {
            this.ThrowDice();
        }

        public void ThrowDice()
        {
            Random random = new Random();
            this.DiceValue = (random.Next(1, 6));
        }

        public int GetDice()
        {
            return (DiceValue);
        }

        bool GetHoldStatus()
        {
            return this.HoldDice;
        }

        public void SetHoldOnDice()
        {
            this.HoldDice = true;
        }

        public void UnSetHoldOnDice()
        {
            this.HoldDice = false;
        }
    }
}


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Yatzy
{
    // DiceCup class
    // Represents DiceCup containing five dices
    // Methods:
    // Throw: Throws dices (whics are not hold)
    // GetDice():          Returns value of dice X
    // SetHold (DiceNo):  SetsHold on dice number X
    // SetHoldToAll ():    Sets all dices on hold
    // UnsetHold (DiceNo): UnHold dice number X
    // UnsetHoldAll ():    No dices are on hold
    // Properties:
    // arrDices: Two dimensional array containg dice(s) and
    //          hold value for each dice (0 = hold, 1 = throw)
    //          Default is hold value set to 1 (throw)

    class DiceCup
    {
        public Dice[] arrDices;
        public int NumberOfDices;

        // Constructor: create x dices       
        public DiceCup(int NumberOfDicesAA)
        {
            NumberOfDices = NumberOfDicesAA;
            Console.Write("NumberOfDices {0}",NumberOfDices);
           
            arrDices = new Dice[NumberOfDices];
            Console.Write("arrDices {0}", arrDices.Length);
        }

        // Throws dices in cup
        public void ThrowDices()
        {
            for (int ArrLoop = 0; ArrLoop < NumberOfDices; ArrLoop++)
            {
                //arrDices[ArrLoop].SetHoldOnDic;
                arrDices[ArrLoop].ThrowDice();
            }
        }

        // Returns values of each dice in aray, starting from Dice1
        public int[] GetDices()
        {
            int[] DicesInCup = new int[NumberOfDices];
            for (int ArrLoop = 1; ArrLoop < NumberOfDices; ArrLoop++)
            //foreach (ArrLoop in arrDices)
            {
                Console.Write("ArrLoop {0}", ArrLoop);
                //DicesInCup[ArrLoop] = arrDices[ArrLoop].GetDice();
                Console.Write(arrDices[ArrLoop].GetDice());
                Console.ReadLine();
            }
            return DicesInCup;
        }

        // Hold all dices
        public void SetHoldToAll()
        {
            for (int ArrLoop = 0; ArrLoop < NumberOfDices; ArrLoop++)
            {
                //arrDices[ArrLoop].SetHoldOnDic;
                arrDices[ArrLoop].SetHoldOnDice();
            }
        }

        // UnsetHold on all dices
        public void UnSetHoldToAll()
        {
            for (int ArrLoop = 0; ArrLoop < NumberOfDices; ArrLoop++)
            {
                arrDices[ArrLoop].UnSetHoldOnDice();
            }
        }
    }
}
Avatar billede tdh1309 Juniormester
18. august 2009 - 21:23 #1
Ups - DiceCup kom med to gange, og min klasse med main kommer nu her:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Yatzy
{
    class Program
    {
        static void Main(string[] args)
        {
            int NumberOfDices = 5;
            int[] arrDiceCount = new int[NumberOfDices];
           
            DiceCup MyCup = new DiceCup(NumberOfDices);
            Console.WriteLine("Rafflebæger er nu oprettet");
           
                       
            arrDiceCount = MyCup.GetDices();
            //Console.WriteLine("Length of row {0}", arrDiceCount.Length);
            /*
            for (int ArrLoop = 1; ArrLoop < NumberOfDices; ArrLoop++)
            {
              // Console.Write(arrDiceCount[ArrLoop]);
            }
           
            */
            Console.ReadLine();
        }
    }
 
}
Avatar billede windcape Praktikant
18. august 2009 - 21:48 #2
Er der nogen speciel grund til at du undlod at fortælle os HVAD fejlen er, istedet for at tro vi er guder som kan læse dine tanker?
Avatar billede windcape Praktikant
18. august 2009 - 21:52 #3
Derudover koder du jo helt forkert. Du bør læse op om properties i C# straks!
Avatar billede windcape Praktikant
18. august 2009 - 21:54 #4
Og her er din fejl:

public DiceCup(int NumberOfDicesAA)
        {
            NumberOfDices = NumberOfDicesAA;
            Console.Write("NumberOfDices {0}", NumberOfDices);

            arrDices = new Dice[NumberOfDices];
            Console.Write("arrDices {0}", arrDices.Length);
        }


At oprette et array af ikke instanced objekter virker ikke.
Avatar billede windcape Praktikant
18. august 2009 - 21:58 #5
Forbedret kode:


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Yatzy
{
    class Program
    {
        static void Main(string[] args)
        {
            DiceCup MyCup = new DiceCup(5);
            Console.WriteLine("Rafflebæger er nu oprettet");
         
            Console.ReadLine();       
        }
    }
   
    class Dice
    {
        private int _value = 1;     
        private bool _hold = false; 
               
        private static Random random = new Random();

        public Dice()
        {
            ThrowDice();
        }

        public void ThrowDice()
        {
            _value = random.Next(1, 6);
        }

        public int Value
        {
            get { return _value; };
        }

        public bool IsHolding
        {
            get { return _hold;  }
            set { _hold = value; }
        }
    }
   
    class DiceCup
    {
        private List<Dice> _dices = new List<Dice>();

        public List<Dice> Dices
        {
            get { return _dices; }
        }

        public DiceCup(int dices)
        {
            for(int i=0;i<dices;i++)
            {
                _dices.Add(new Dice());
            }
        }

        public void ThrowDices()
        {
            foreach(var dice in _dices)
            {
                dice.ThrowDice();
            }
        }

        public void SetHoldToAll()
        {
            foreach(var dice in _dices)
            {
                dice.IsHolding = true;
            }
        }

        public void UnSetHoldToAll()
        {
            foreach(var dice in _dices)
            {
                dice.IsHolding = false;
            }
        }
    }       
}
Avatar billede windcape Praktikant
18. august 2009 - 22:08 #7
> At oprette et array af ikke instanced objekter virker ikke.

Lidt dybere forklaring:

Dice[] diceArray = new Dice[5];


Her har du 5x Dices som er ikke er instansiseret. Det kan gøres med:

for(int i=0;i<diceArray.Length;i++)
{
    diceArray[i] = new Dice();
}


Alternativt kan du benytte dig af Lists:

List<Dice> diceList = new List<Dice>();

Her er der så en tom liste af terninger, som du kan sætte ind i:

for(int i=0;i<5;i++)
{
    diceList.Add(new Dice());
}


Det simplificere også resten af dit program at benytte lists.
Avatar billede windcape Praktikant
18. august 2009 - 22:10 #8
Afsluttede kommentar:

Det koster memory og performance at oprette et Random() objekt for hver Dice instans. Derfor bør du definere den som statisk, hvilket gør at der kun er een for hele dit program.

private static Random random = new Random();


Det er både hurtigere, og giver mindre RAM forbrug.
Avatar billede Syska Mester
18. august 2009 - 22:41 #9
windcape:
Som svar til din første kommentar ... selvf skal han tro vi er guder ... man skal have høje tanker om sig selv ... :-) og tro at man kan alt ... det kan man så senere finde ud af at man måske ikke kan :-)

// ouT
Avatar billede tdh1309 Juniormester
18. august 2009 - 22:43 #10
Mange tak for kommentarer og hjælp!
Er godt klar over at jeg har lidt at læse op på :-)

Kom med et svar så du kan få dine velfortjente point :-)
Avatar billede bitmatic Nybegynder
18. august 2009 - 22:57 #11
Det er måske lige i overkanten at påstå at han koder helt forkert, fordi han ikke bruger ret mange properties.... Der er godt nok mange mennesker der mener at alt skal laves med properties, men jeg kan finde mange argumenter imod at bruge properties.

Det afhænger helt af situationen.
Avatar billede Syska Mester
18. august 2009 - 23:08 #12
Helt klart ...

Jeg synes nu også windcape giver lidt for meget gas for at fortælle ham at han ikke koder helt genialt ... :-) Kunne godt pakkes lidt pænere ind ...

Han skriver trods alt i første linje at han er ny :-)

// ouT
Avatar billede windcape Praktikant
18. august 2009 - 23:12 #13
> Det er måske lige i overkanten at påstå at han koder helt forkert

At skrive getters/setters når C# har specifik syntaks til det, er ihvertfald "forkert". Der er defacto code-conventions osv. og da det ikke er Java vi koder i, bør vi benytter properties.

> Kunne godt pakkes lidt pænere ind

Ikke min afdeling :p
Avatar billede windcape Praktikant
18. august 2009 - 23:14 #14
> Som svar til din første kommentar

Pointen er bare at jeg kunne have besvaret spørgsmålet uden at skulle kompilere det selv, hvis han havde skrevet at han fik en InstanceNotFoundException på 'arrDices[ArrLoop].ThrowDice();' linjen.
Avatar billede Syska Mester
18. august 2009 - 23:26 #15
Korrekt ... det kunne være bedre stillet hans sprøgsmål ...

Men i stedet for at skrive han koder helt forkert er min eneste pointe at du kunne kommentere at han nok burde læse om på Properties :-)

Nej, det er ikke din afdeling at være venlig venlig her... men måske vi kunne have en indflydelse på det.

og ja ... jeg er godt klar over din pointe ... fuldt forståeligt, men jeg må antage at du alligevel ikke har haft andet at give dig til siden du alligevel compiler det hele selv :-)

Men udover de første 2 posts ... så er resten jo ikke mindre end genialt for sprøger ... stor ros herfra :-)
Avatar billede arne_v Ekspert
19. august 2009 - 01:48 #16
re Random instantiering)

Det er aldeles ligegyldigt for memory og CPU forbrug at man instantierer en instans af Random for hver Dice.

Det er en katastrofe for funktionaliteten at have en instans af Random for hver Dice. Man er næsten sikker på at få 5 ens. Og det er jo næppe hensigten.
Avatar billede windcape Praktikant
19. august 2009 - 02:01 #17
Hvorfor? Mig bekendt afhænger resultatet ikke af hvornår du opretter objektet, men hvornår du kalder Next(h,t);

Ved 100 terninger er forskellen er bare om du vil have 100x Random instanser i memory, eller een.
Avatar billede arne_v Ekspert
19. august 2009 - 02:52 #18
Resultatet afhænger af hvornår du opretter Random objektet.

Hvis resultatet afhang af hvornår du kaldte Next så ville det jo være umuligt at initialisere et array med random data hurtigt.

Det er grunden til at man aldrig bør instantiere mane Random objekter lige efter hinanden.

Og den nemmeste måde at undgå det på er ved kun at have et enkelt objekt.
Avatar billede arne_v Ekspert
19. august 2009 - 04:31 #19
Og det kan godt være svær at få kode skrevet rigtig.

Vi kan jo tage den lille øvelse: hvorfor er A langt bedre kode men også dårligere design end B her:

A:

        private List<Dice> _dices = new List<Dice>();
        public List<Dice> Dices
        {
            get { return _dices; }
        }

B:

        private Dice[] arrDices;
        public int[] GetDices()
        {
            int[] DicesInCup = new int[NumberOfDices];
            for (int ArrLoop = 0; ArrLoop < NumberOfDices; ArrLoop++)
            {
                DicesInCup[ArrLoop] = arrDices[ArrLoop];
            }
            return DicesInCup;
        }
Avatar billede windcape Praktikant
19. august 2009 - 06:36 #20
Du sætter int = Dice i eksempel B. I tilfælde at det skulle have været dens Value, så er forskellen vel at du kun returnere værdierne af terningerne, som måske kan være en god ting.

Kan dog ikke fange pointen?

> Resultatet afhænger af hvornår du opretter Random objektet.

Jeg testede koden, og det giver nu stadigvæk forskellige værdier på terningerne.
Avatar billede arne_v Ekspert
19. august 2009 - 16:15 #21
Ja - der skal vist hentes .Value.

Pointen er at ved at returnere en referance til den interne liste, saa bryder man delvist encapsulation. Kode udenfor DiceCup kan nemlig aendre i listen. Den originale kode (hvis den blev tilrettet til at virke) returnerede en kopi af data og havde derfor ikke dette ptoblem.
Avatar billede arne_v Ekspert
19. august 2009 - 16:36 #22
Med hensyn til Random - hvad giver foelgende hos dig:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;

namespace Yatzy
{
    class Program
    {
        static void Main(string[] args)
        {
            for(int i = 0; i < 5; i++)
            {
                DiceCup1 MyCup1 = new DiceCup1(5);
                Console.Write("Multi Random 1: ");
                foreach(Dice1 d in MyCup1.Dices)
                {
                    Console.Write(" " + d.Value);
                }
                Console.WriteLine();
                DiceCup2 MyCup2 = new DiceCup2(5);
                Console.Write("Multi Random 2: ");
                foreach(Dice2 d in MyCup2.Dices)
                {
                    Console.Write(" " + d.Value);
                }
                Console.WriteLine();
                DiceCup3 MyCup3 = new DiceCup3(5);
                Console.Write("Single Random : ");
                foreach(Dice3 d in MyCup3.Dices)
                {
                    Console.Write(" " + d.Value);
                }
                Console.WriteLine();
                Thread.Sleep(100);
            }
            Console.ReadLine();     
        }
    }
    class Dice1
    {
        private int _value;
        public Dice1()
        {
            ThrowDice();
        }
        public void ThrowDice()
        {
            Random random = new Random();
            _value = random.Next(1, 6);
        }
        public int Value
        {
            get { return _value; }
        }
    }
    class DiceCup1
    {
        private List<Dice1> _dices = new List<Dice1>();
        public List<Dice1> Dices
        {
            get { return _dices; }
        }

        public DiceCup1(int dices)
        {
            for(int i=0;i<dices;i++)
            {
                _dices.Add(new Dice1());
            }
        }
    }     
    class Dice2
    {
        private Random random = new Random();
        private int _value;
        public Dice2()
        {
            ThrowDice();
        }
        public void ThrowDice()
        {
            _value = random.Next(1, 6);
        }
        public int Value
        {
            get { return _value; }
        }
    }
    class DiceCup2
    {
        private List<Dice2> _dices = new List<Dice2>();
        public List<Dice2> Dices
        {
            get { return _dices; }
        }

        public DiceCup2(int dices)
        {
            for(int i=0;i<dices;i++)
            {
                _dices.Add(new Dice2());
            }
        }
    }     
    class Dice3
    {
        private static Random random = new Random();
        private int _value;
        public Dice3()
        {
            ThrowDice();
        }
        public void ThrowDice()
        {
            _value = random.Next(1, 6);
        }
        public int Value
        {
            get { return _value; }
        }
    }
    class DiceCup3
    {
        private List<Dice3> _dices = new List<Dice3>();
        public List<Dice3> Dices
        {
            get { return _dices; }
        }

        public DiceCup3(int dices)
        {
            for(int i=0;i<dices;i++)
            {
                _dices.Add(new Dice3());
            }
        }
    }     
}
Avatar billede windcape Praktikant
19. august 2009 - 17:50 #23
> Pointen er at ved at returnere en referance til den interne liste, saa bryder man delvist encapsulation.

Det er rigtigt. Jeg ville dog benytte mig af en iterator istedet for et array med values. Og da Dice's value er read-only, er det vel ikket problem.

[code]
    class DiceCup
    {
        private List<Dice> _dices = new List<Dice>();

        public IEnumerable<Dice> Dices
        {
            get
            {
                foreach(var dice in _dices)
                {
                    yield return dice;
                }
            }
        }
[/code]
Avatar billede windcape Praktikant
19. august 2009 - 17:55 #24
Uups, det har samme problem. Bedre løsning:


    class DiceCup
    {
        private List<Dice> _dices = new List<Dice>();

        public IEnumerable<int> Dices
        {
            get
            {
                foreach(var dice in _dices)
                {
                    yield return dice.Value;
                }
            }
        }
Avatar billede windcape Praktikant
19. august 2009 - 18:06 #25
> Med hensyn til Random - hvad giver foelgende hos dig:

Nu er jeg forvirret. Den kode som essentielt giver det bedste resultat, er hvor man benytter en static random.

Jeg troede det netop var det jeg skrev/foreslog tidligere, men som var forkert?
Avatar billede arne_v Ekspert
19. august 2009 - 18:24 #26
Det var en udmaerket maade at goere det paa.

Et alternativ var:

        public IList<Dice> Dices
        {
            get { return _dices.AsReadOnly(); }
        }
Avatar billede arne_v Ekspert
19. august 2009 - 18:25 #27
Dit forslag om en static Random var helt rigtigt. Men din argumentation for den rigtige loesning var forkert.
Avatar billede arne_v Ekspert
19. august 2009 - 18:50 #28
Ioevrigt er der i sig selv noget godt ved at returnere IList<> fremfor List<>, da det giver lidt frihed til at skifte implementation.

Det er traditionelt ikke en synsvinkel som man prioriterer i C# verdenen. Men I Java verdenen ville du faa bank for at lade en metode returnere ArrayList<> fremfor List<>.
Avatar billede tdh1309 Juniormester
19. august 2009 - 21:16 #29
Hej Alle
Tak for de mange kommentarer og forslag til hvor jeg kan hente viden! Super.
Vedr. min manglende beskrivelse af fejl - jeg fik ikke nogen descideret fejlmeddelelse! Fejlen opstod ved program afvikling, og der kom ikke noget fornuftigt. Men ja - jeg kunne godt have beskrevet det lidt bedre, det vil ske næste gang.

Endnu en gang tak til alle for deltagelsen i mit problem :-)

/Thomas
Avatar billede Syska Mester
19. august 2009 - 22:02 #30
Bare det at det var en Runtime fejl ville have hjulpet.
Avatar billede windcape Praktikant
20. august 2009 - 01:21 #31
> Vedr. min manglende beskrivelse af fejl - jeg fik ikke nogen
> descideret fejlmeddelelse! Fejlen opstod ved program afvikling

Oh, du havde ikke kompileret som debug? Min debugger i Visual Studio fortalte mig helst præcist hvad fejlen var, og hvor den var, ligeså snart jeg kørte programmet.
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