Sign in to follow this  
Diosces

C# basic function calls--two different ways, Which is better?

Recommended Posts

Well I'm working some problems in one of my C# texts and I'm practicing redoing an exercise without looking at the source code. It's a windows form program where guys exchange cash with themselves and a bank. When I wrote the button code for the two guys to exchange cash it came out different from what the book's solution is. I know there are more than one way's to get a result. My code, I believe, works exactly as the books. I'd appreciate any comments on whether my function call is 'preferred' over the book's method or vice versa. I put the the suggested book's solution in //comments. The call is at the bottom of the form "bob.Cash where the result is Joe gives $10 to Bob.
using System;
using System.Windows.Forms;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;


namespace WindowsFormsApplication1
{
    public partial class Form1 : Form
    {
        guy joe;
        guy bob;
        int bank=100;

        public Form1()
        {
            InitializeComponent();
            joe = new guy();
            joe.Cash=50;
            joe.name="Joe";
            bob = new guy();
            bob.Cash = 50;
            bob.name = "Bob";

            UpdateForm();
        }

        public void UpdateForm()
        {
            joescash.Text = joescash.Name + "has $" + joe.Cash;
            bobscash.Text = bobscash.Name + "has $" + bob.Cash;
            bankcash.Text = bankcash.Name + "has $" + bank;
        }


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

        private void button1_Click(object sender, EventArgs e)
        {
            if (bank >= 10)
            {
                bank -= joe.ReceiveCash(10);
                UpdateForm();
            }
            else
            {
                MessageBox.Show("The bank is low on money");
            }
        }

        private void button2_Click(object sender, EventArgs e)
        {
            bank += bob.GiveCash(5);
            UpdateForm();
        }

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

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

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

        private void button3_Click(object sender, EventArgs e)
        {
            bob.Cash += joe.GiveCash(10);
            UpdateForm();
            //could also write:
            //bob.ReceiveCash(joe.GiveCash(10))
            //UpdateForm();
        }

        private void button4_Click(object sender, EventArgs e)
        {
            joe.Cash += bob.GiveCash(5);
            UpdateForm();
        }
    }
}

Note the form calls the guy class functions from this class form
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Windows.Forms;


namespace WindowsFormsApplication1
{
    public class guy
    {
        public int Cash;
        public string name;

        public int GiveCash(int amount)
        {
            if (amount <= Cash && amount > 0)
            {
                Cash -= amount;
                return amount;
            }
            else
            {
                MessageBox.Show("I don't have enough cash to give you " + amount);
                return 0;
            }
        }
        public int ReceiveCash(int amount)
        {
            if (amount > 0)
            {
                Cash += amount;
                return amount;
            }
            else
            {
                MessageBox.Show(amount + "isn't an amount I'll take", name + "says");
                return 0;
            }

        }
    }
}



Appreciate any feedback, as I said I want to make sure I don't develop sloppy coding habits. Thanks kindly

Share this post


Link to post
Share on other sites
Generally, directly manipulating data like you're doing is ill-advised; especially since there's a separate method available to do it. That said, the Cash variable is likely to be private/protected in that sort of arrangement. Another implementation might want you to directly interact with the Cash variable, but would make it a property, doing the error handling there. That is a bit more C#-like too.

In general though, the guy class is bleh even for throwaway book code. Doing error checking on half of the transaction at a time is fraught with problems.

And as a slight aside, the variable naming should be more consistent and C# like. It's not a great big deal, but makes the maintenance of code easier and makes the code more polished looking which helps for morale.

Share this post


Link to post
Share on other sites
Hi,

I would prefer the book's solution, it looks a little bit more object-oriented and is quite good understandable. Direct access to the members of an object from another object's scope is normally not the best choice.

I would make the Cash and name members of class guy private and I would provide getters and setters to access them.

Best regards

Share this post


Link to post
Share on other sites
Thanks for the feedback guys.

Tela I try to keep that all in mind. I think a quarter of of it went over my head.

Porthos, thanks I will try the getters and setters..Very good point.
As for "Direct access to the members of an object from another object's scope is normally not the best choice."--The book makes that their choice(books code is in comments). Mine increments off the GiveCash function which I thought was safer..

Share this post


Link to post
Share on other sites
But it increments bob.Cash directly. If Cash were a property, that might be okay. Since it's not (and the design of the class tends toward it not being property-like) its into the realm of 'less than ideal'.

The book does not directly manipulate the member, it instead uses the provided RecieveCash which does more error checking and provides a since entry to adding to Cash (if Cash were private like it is supposed to be in that sort of design).

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this