How compact is this MS Visual studio C# code?

I made a "game" and I'd like to have feedback on how compact my code is. As in, I should of done 'x' instead of 'y'. Or it's better to 'x' than to 'y', just general feedback. Also, I'm not sure if I hard coded or not so i'd like to know if I did or not too.

virus scan # 1 accadantly went to german site
virus scan #2

Even if you don't want to review it, download it anyways for the awesome(read: horribly laughable) pixel art.

Robots_mpgh.net.rar187 KB · 9 downloads Scanning…

Download & Thanks

Can't wait to try it. It should be approved today sometime. @Psychotic

"
Last Activity
5 Hours Ago
"
post is 10 hours old

: D

/approved .

2 unhandled exceptions.
1 when you dont choose robots and the other when not choosing an attack.
No biggy..

Originally Posted by Pingo

2 unhandled exceptions.
1 when you dont choose robots and the other when not choosing an attack.
No biggy..

Oh yea that haha, but the code is good yes? Like, no problems in the way I tackled the problem ( should've chosen Inheritance, Put the code in a class to handle it better etc.) right?

I'll have a look see at the code in the morning.
Let you know what i think. Choob opinion!

Upload regular source, not an installer?

Originally Posted by Pingo

I'll have a look see at the code in the morning.
Let you know what i think. Choob opinion!

Alright thanks!

---------- Post added at 10:07 PM ---------- Previous post was at 10:06 PM ----------

Originally Posted by abuckau907

Upload regular source, not an installer?

I thought with the installer you could just open the solution in Visual Studio and then browse through it very easily.

And I thought the .rar would have files with source code, not executables : ( o well, thanks for commenting back. If you post/upload a plain-text version of the source, I'll look it over.

Originally Posted by abuckau907

And I thought the .rar would have files with source code, not executables : ( o well, thanks for commenting back. If you post/upload a plain-text version of the source, I'll look it over.

Alright but bear in mind, it could get confusing. Here goes

Code:

interface IRobot
    {
        int Fuel { get; set; }
        string[] Attacks { get;  set; }
        void ConsumeFuel(int difficultylevel);
       
        void Fight(string Attack, IRobot Target);
        
    }
class FlyingRobot : IRobot
    {
        public int Fuel { get; set; }
        public string[] Attacks { get; set ; }

        public FlyingRobot()
        {
            
            Attacks = new string[3];
            Attacks[0] = "Tackle"; Attacks[1] = "Wing Attack"; Attacks[2] = "Flying Kick";
        }

        public void ConsumeFuel(int difficultylevel)
        {
            if (difficultylevel == 1)
                Fuel -= 50;
            if (difficultylevel == 2)
                Fuel -= 100;
            if (difficultylevel == 3)
                Fuel -= 200;

        }

        public void Fight(string Attack, IRobot Target)
        {
            for (int i = 0; i < Attacks.Length; i++)
            {
                if (Attacks[i].Contains(Attack))
                {
                    if (Attack == "Tackle")
                    {
                        ConsumeFuel(1); Target.Fuel -= 100;
                    }
                    if (Attack == "Wing Attack")
                    {
                        ConsumeFuel(2); Target.Fuel -= 200;
                    }
                    if (Attack == "Flying Kick")
                    {
                        ConsumeFuel(3); Target.Fuel -= 400;
                    }
                }
            }

        }
    }
class BoxingRobot : IRobot
    {
         public int Fuel { get; set; }
        public string[] Attacks { get; set ; }

        public BoxingRobot()
        {
            
            Attacks = new string[3];
            Attacks[0] = "Tackle"; Attacks[1] = "Punch"; Attacks[2] = "Upper Cut";
        }

        public void ConsumeFuel(int difficultylevel)
        {
            if (difficultylevel == 1)
                Fuel -= 50;
            if (difficultylevel == 2)
                Fuel -= 100;
            if (difficultylevel == 3)
                Fuel -= 200;

        }

        public void Fight(string Attack, IRobot Target)
        {
            for (int i = 0; i < Attacks.Length; i++)
            {
                if (Attacks[i].Contains(Attack))
                {
                    if (Attack == "Tackle")
                    {
                        ConsumeFuel(1); Target.Fuel -= 100;
                    }
                    if (Attack == "Punch")
                    {
                        ConsumeFuel(2); Target.Fuel -= 200;
                    }
                    if (Attack == "Upper Cut")
                    {
                        ConsumeFuel(3); Target.Fuel -= 400;
                    }
                }
            }

        }
    }
class KickingRobot : IRobot
    {
         public int Fuel { get; set; }
        public string[] Attacks { get; set ; }

        public KickingRobot()
        {
            
            Attacks = new string[3];
            Attacks[0] = "Tackle"; Attacks[1] = "Kick"; Attacks[2] = "Taekwondo Kick";
        }

        public void ConsumeFuel(int difficultylevel)
        {
            if (difficultylevel == 1)
                Fuel -= 50;
            if (difficultylevel == 2)
                Fuel -= 100;
            if (difficultylevel == 3)
                Fuel -= 200;

        }

        public void Fight(string Attack, IRobot Target)
        {
            for (int i = 0; i < Attacks.Length; i++)
            {
                if (Attacks[i].Contains(Attack))
                {
                    if (Attack == "Tackle")
                    {
                        ConsumeFuel(1); Target.Fuel -= 100;
                    }
                    if (Attack == "Kick")
                    {
                        ConsumeFuel(2); Target.Fuel -= 200;
                    }
                    if (Attack == "Taekwondo Kick")
                    {
                        ConsumeFuel(3); Target.Fuel -= 400;
                    }
                }
            }

        }
    }
class DiggingRobot : IRobot
    {
         public int Fuel { get; set; }
        public string[] Attacks { get; set ; }

        public DiggingRobot()
        {
            
            Attacks = new string[3];
            Attacks[0] = "Tackle"; Attacks[1] = "Dig"; Attacks[2] = "Dig Attack";
        }

        public void ConsumeFuel(int difficultylevel)
        {
            if (difficultylevel == 1)
                Fuel -= 50;
            if (difficultylevel == 2)
                Fuel -= 100;
            if (difficultylevel == 3)
                Fuel -= 200;

        }

        public void Fight(string Attack, IRobot Target)
        {
            for (int i = 0; i < Attacks.Length; i++)
            {
                if (Attacks[i].Contains(Attack))
                {
                    if (Attack == "Tackle")
                    {
                        ConsumeFuel(1); Target.Fuel -= 100;
                    }
                    if (Attack == "Dig")
                    {
                        ConsumeFuel(2); Target.Fuel -= 200;
                    }
                    if (Attack == "Dig Attack")
                    {
                        ConsumeFuel(3); Target.Fuel -= 400;
                    }
                }
            }

        }
    }
class Controller
    {

        ComboBox combo1, combo2;

        public IRobot one, two;
        public Controller(ComboBox firstcombobox,ComboBox secondcombobox, string one,string two, int startingfuel)
        {
            if (one.Contains("Fly"))
                this.one = new FlyingRobot();
            if (one.Contains("Kick"))
                this.one = new KickingRobot();
            if (one.Contains("Box"))
                this.one = new BoxingRobot();
            if (one.Contains("Dig"))
                this.one = new DiggingRobot();
            if (two.Contains("Fly"))
                this.two = new FlyingRobot();
            if (two.Contains("Kick"))
                this.two = new KickingRobot();
            if (two.Contains("Box"))
                this.two = new BoxingRobot();
            if (two.Contains("Dig"))
                this.two = new DiggingRobot();

            this.one.Fuel = startingfuel;
            this.two.Fuel = startingfuel;
            combo1 = firstcombobox; combo2 = secondcombobox;

            for (int i = 0; i < this.one.Attacks.Length; i++)
            {
                combo1.Items.Add(this.one.Attacks[i]);
                combo2.Items.Add(this.two.Attacks[i]);
            }
            
        }
        public void fight()
        {
            one.Fight(combo1.SelectedItem.ToString(), two);
            two.Fight(combo2.SelectedItem.ToString(), one);
        }
    }
public static string IsBelowZero(TextBox firsttextbox, TextBox secondtextbox,Controller controller)
        {
            
            for (int i = -1; i > -10000; i--)
            {
                if (int.Parse(firsttextbox.Text) == i)
                {
                    if (controller.two is DiggingRobot) return "Digging Robot(2) has won!";
                    if (controller.two is KickingRobot) return "Kicking Robot(2) has won!";
                    if (controller.two is BoxingRobot) return "Boxing Robot(2) has won!";
                    if (controller.two is FlyingRobot) return "Flying Robot(2) has won!";

                }
                if (int.Parse(secondtextbox.Text) == i)
                {
                    if (controller.one is DiggingRobot) return "Digging Robot(1) has won!";
                    if (controller.one is KickingRobot) return "Kicking Robot(1) has won!";
                    if (controller.one is BoxingRobot) return "Boxing Robot(1) has won!";
                    if (controller.one is FlyingRobot) return "Flying Robot(1) has won!";
                }
            }
            return "Nobody";
        }
public partial class Form1 : Form
    {
        Controller controller;
        Image Digger1,Digger2, Kicker1,Kicker2, Boxer1,Boxer2, Flyer1,Flyer2;

        string firstopponent, secondopponenet;
        public Form1()
        {
            InitializeComponent();
            
           
            
        }

        void Update()
        {
            textBox1.Text = controller.one.Fuel.ToString();
            textBox2.Text = controller.two.Fuel.ToString();
            if (StaticClass.IsBelowZero(textBox1, textBox2, controller).Contains("won"))
            {
                MessageBox.Show(StaticClass.IsBelowZero(textBox1, textBox2, controller));
                
            }
        }
        

       

        private void button1_Click(object sender, EventArgs e)
        {
            controller.fight();
            Update();
        }

        private void button2_Click(object sender, EventArgs e)
        {
           
            RadioButton[] radiobuttonarray1 = new RadioButton[] { radioButton1, radioButton2, radioButton3, radioButton7 };
            RadioButton[] radiobuttonarray2 = new RadioButton[] { radioButton4, radioButton5, radioButton6, radioButton8 };
            for (int i = 0; i < radiobuttonarray1.Length; i++)
            {
                if (radiobuttonarray1[i].Checked)
                    firstopponent = radiobuttonarray1[i].Text;
                if (radiobuttonarray2[i].Checked)
                    secondopponenet = radiobuttonarray2[i].Text;
            }

            controller = new Controller(comboBox1, comboBox2, firstopponent, secondopponenet, 10000);
            pictureBox1.Visible = false; pictureBox4.Visible = false; pictureBox2.Visible = true; pictureBox5.Visible = true;
            textBox1.Visible = true; textBox2.Visible = true;
            comboBox1.Visible = true; comboBox2.Visible = true;
            button1.Visible = true; label1.Visible = true;
            
            groupBox1.Visible = false; groupBox2.Visible = false; label2.Visible = false; button2.Visible = false;

            Update();
        }

        private void radioButton1_CheckedChanged(object sender, EventArgs e)
        {
            
            pictureBox1.Image = Digger1;
            
            pictureBox2.Image = Digger2;
        }

        private void radioButton2_CheckedChanged(object sender, EventArgs e)
        {
            pictureBox1.Image = Kicker1;
            pictureBox2.Image = Kicker2;
        }

        private void radioButton3_CheckedChanged(object sender, EventArgs e)
        {
            pictureBox1.Image = Boxer1;
            pictureBox2.Image = Boxer2;
        }

        private void radioButton7_CheckedChanged(object sender, EventArgs e)
        {
            pictureBox1.Image = Flyer1;
            pictureBox2.Image = Flyer2;
        }

        private void radioButton6_CheckedChanged(object sender, EventArgs e)
        {
            pictureBox4.Image = Digger1;
            pictureBox5.Image = Digger2;
        }

        private void radioButton5_CheckedChanged(object sender, EventArgs e)
        {
            pictureBox4.Image = Kicker1;
            pictureBox5.Image = Kicker2;
        }

        private void radioButton4_CheckedChanged(object sender, EventArgs e)
        {
            pictureBox4.Image = Boxer1;
            pictureBox5.Image = Boxer2;
        }

        private void radioButton8_CheckedChanged(object sender, EventArgs e)
        {
            pictureBox4.Image = Flyer1;
            pictureBox5.Image = Flyer2;
        }

        private void Form1_Load(object sender, EventArgs e)
        {
            pictureBox1.Visible = true; pictureBox4.Visible = true;
            
            Digger1 = Properties.Resources.DiggerRobot1; Digger2 = Properties.Resources.DiggerRobot2;
            Kicker1 = Properties.Resources.KickingRobot1; Kicker2 = Properties.Resources.KickingRobot2;
            Boxer1 =  Properties.Resources.BoxerRobot1; Boxer2 = Properties.Resources.BoxerRobot2;
            Flyer1 = Properties.Resources.FlyingRobot1; Flyer2 = Properties.Resources.FlyingRobot2;
        }
    }

Welp , that's all! I added a new class so that it displays a message when one of them wins in this version :P

Looks really good. (Using inheritance vs interface can be a matter of choice: until the project is a little bigger, one doesn't necessarily have an advantage over the other)

Only part that sticks out to me is

Code:

                    if (Attack == "Tackle")
                    {
                        ConsumeFuel(1); Target.Fuel -= 100;
                    }
                    if (Attack == "Dig")
                    {
                        ConsumeFuel(2); Target.Fuel -= 200;
                    }
                    if (Attack == "Dig Attack")
                    {
                        ConsumeFuel(3); Target.Fuel -= 400;
                    }

Because 'the same code' appears in all of the robot's attack function. - inheritance?
It's not a problem, but from past experience having duplicate code means you'll update it in 1 location and forget to update it in the second.
If you plan to add more 'attack things' maybe create a structure/class to represent 'attack spell'. ?

^^ I like it.

Originally Posted by abuckau907

Code:

                    if (Attack == "Tackle")
                    {
                        ConsumeFuel(1); Target.Fuel -= 100;
                    }
                    if (Attack == "Dig")
                    {
                        ConsumeFuel(2); Target.Fuel -= 200;
                    }
                    if (Attack == "Dig Attack")
                    {
                        ConsumeFuel(3); Target.Fuel -= 400;
                    }

Because 'the same code' appears in all of the robot's attack function. - inheritance?

Thanks for the advice! The reason I didn't choose inheritance for that is because in each of the classes there are different attacks , therefore they have to check for different attacks. So one would check for Tackle,Dig, Digattack and the other would check for Tackle,Kick,TaekwondoKick. So I'd have to override each of the methods either way.

Looks fine from a quick glance.

One thing i do when dealing with alot of the same type of control.
Make them share the same event so i can keep the code together.

I might be wrong in doing this but i choose to do it like this.
Example...

Instead of this

Code:

        private void radioButton1_CheckedChanged(object sender, EventArgs e)
        {
            
            pictureBox1.Image = Digger1;
            
            pictureBox2.Image = Digger2;
        }

        private void radioButton2_CheckedChanged(object sender, EventArgs e)
        {
            pictureBox1.Image = Kicker1;
            pictureBox2.Image = Kicker2;
        }

        private void radioButton3_CheckedChanged(object sender, EventArgs e)
        {
            pictureBox1.Image = Boxer1;
            pictureBox2.Image = Boxer2;
        }

        private void radioButton7_CheckedChanged(object sender, EventArgs e)
        {
            pictureBox1.Image = Flyer1;
            pictureBox2.Image = Flyer2;
        }

        private void radioButton6_CheckedChanged(object sender, EventArgs e)
        {
            pictureBox4.Image = Digger1;
            pictureBox5.Image = Digger2;
        }

        private void radioButton5_CheckedChanged(object sender, EventArgs e)
        {
            pictureBox4.Image = Kicker1;
            pictureBox5.Image = Kicker2;
        }

        private void radioButton4_CheckedChanged(object sender, EventArgs e)
        {
            pictureBox4.Image = Boxer1;
            pictureBox5.Image = Boxer2;
        }

        private void radioButton8_CheckedChanged(object sender, EventArgs e)
        {
            pictureBox4.Image = Flyer1;
            pictureBox5.Image = Flyer2;
        }

I would do this to keep the code together for easier modifications.

Code:

        void RB_CheckedChanged(object sender, EventArgs e)
        {
            if (((RadioButton)sender).Checked)
            {
                switch (((RadioButton)sender).Name)
                {
                    case "radioButton1":
                        pictureBox1.Image = Digger1;
                        pictureBox2.Image = Digger2;
                        break;

                    case "radioButton2":
                        pictureBox1.Image = Kicker1;
                        pictureBox2.Image = Kicker2;
                        break;

                    case "radioButton3":
                        pictureBox1.Image = Boxer1;
                        pictureBox2.Image = Boxer2;
                        break;

                    case "radioButton4":
                        pictureBox4.Image = Boxer1;
                        pictureBox5.Image = Boxer2;
                        break;

                    case "radioButton5":
                        pictureBox4.Image = Kicker1;
                        pictureBox5.Image = Kicker2;
                        break;

                    case "radioButton6":
                        pictureBox4.Image = Digger1;
                        pictureBox5.Image = Digger2;
                        break;

                    case "radioButton7":
                        pictureBox1.Image = Flyer1;
                        pictureBox2.Image = Flyer2;
                        break;

                    case "radioButton8":
                        pictureBox4.Image = Flyer1;
                        pictureBox5.Image = Flyer2;
                        break;
                }
            }
        }

And you would add this to your form load event.

Code:

        private void Form1_Load(object sender, EventArgs e)
        {
            pictureBox1.Visible = true; pictureBox4.Visible = true;          
            Digger1 = Properties.Resources.DiggerRobot1; Digger2 = Properties.Resources.DiggerRobot2;
            Kicker1 = Properties.Resources.KickingRobot1; Kicker2 = Properties.Resources.KickingRobot2;
            Boxer1 =  Properties.Resources.BoxerRobot1; Boxer2 = Properties.Resources.BoxerRobot2;
            Flyer1 = Properties.Resources.FlyingRobot1; Flyer2 = Properties.Resources.FlyingRobot2;
            foreach (Control C in this.Controls)
            {
                if (C is RadioButton)
                {
                    ((RadioButton)C).CheckedChanged += new System.EventHandler(RB_CheckedChanged);
                }
            }
        }

Originally Posted by Pingo

Thanks for the suggestion! Altough I do understand the code, I don't really understand how the object sender and events etc. work yet. I also haven't gotten to the point in the book i'm learning from where I learn about switch and case, even though I understand how it works. But since I don't know how those things work I don't think i'll be able to do this method. But when I do know more about them I'll definitly go back to this thread to try them out!

How compact is this MS Visual studio C# code?

Post a Reply

Similar Threads

Tags for this Thread