Home | Contact Us | FAQ | Search & Site Map | Link to Us
Sign In | Join | Other 45 Sites in Network
HomeAnnouncementsFree MagazinesWhite PapersSubmit Content
Discussion GroupsASP.NETWindows FormsLanguages.NET FrameworkVisual Studio.NET
Articles.NET FrameworkASP.NETToolsWindows Forms
.NET DirectoryOpen Source ProjectsUser GroupsWeb Resources
Related Topics
Visual Basic 6SQL ServerMS AccessOther DB ProductsMS Server ProductsMore Topics ...

.NET Forum / Windows Forms / WinForm General / June 2007

Tip: Looking for answers? Try searching our database.

My first C# project

Thread view: 
Enable EMail Alerts  Start New Thread
Thread rating: 
Arjen Logghe - 11 Jun 2007 21:35 GMT
Hello all,

I've written my first real C# program. I am posting this here, because
I would like your opinion. Is it good, bad, where can I improve etc.?
The project is a simple calculator, I've got the basics working and
I'll probably extend its functionality with a filelogger and
memorybuttons to make it more of a real calculator. I look forward
from hearing from you.

Regards,

A.Logghe.

Here's the code:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Text;
using System.Windows.Forms;

namespace Calculator
{

   public partial class MyCalculator : Form
   {
       private string firstOperand = "";
       private string secondOperand = "";
       private bool isFirstChar = true;
       private bool CommaChrUsed = false;
       private int operandCount = 0;
       private string operandType = "";

       public MyCalculator()
       {
           InitializeComponent();
       }

       private void btnNumeral_Click(object sender, EventArgs e) //
buttons (0-9 + comma)
       {
           Button b = new Button();
           b = (Button)sender;

           if (operandCount == 0)
           {
               if (isFirstChar)
               {
                   if (b.Text == "," || b.Text == "0")
                   {
                       firstOperand = "0,";
                       CommaChrUsed = true;
                       isFirstChar = false;
                   }
                   else
                   {
                       firstOperand += b.Text;
                       txtBoxDisplay.Text = firstOperand;
                       isFirstChar = false;
                   }
               }
               else //!isFirstChar
               {
                   if (b.Text == "," && !CommaChrUsed)
                   {
                       firstOperand += b.Text;
                       txtBoxDisplay.Text = firstOperand;
                       CommaChrUsed = true;
                   }

                   if (b.Text != ",")
                   {
                       firstOperand += b.Text;
                       txtBoxDisplay.Text = firstOperand;
                   }
               }
           }
           else //operandCount != 0
           {
               if (isFirstChar)
               {
                   if (b.Text == "," || b.Text == "0")
                   {
                       secondOperand = "0,";
                       CommaChrUsed = true;
                       isFirstChar = false;
                   }
                   else
                   {
                       secondOperand += b.Text;
                       txtBoxDisplay.Text = secondOperand;
                       isFirstChar = false;
                   }
               }
               else //!isFirstChar
               {
                   if (b.Text == "," && !CommaChrUsed)
                   {
                       secondOperand += b.Text;
                       txtBoxDisplay.Text = secondOperand;
                       CommaChrUsed = true;
                   }

                   if (b.Text != ",")
                   {
                       secondOperand += b.Text;
                       txtBoxDisplay.Text = secondOperand;
                   }
               }
           }
       }

       private void btnOperator_Click(object sender, EventArgs e) //
all the standard arithmetic operators + %-operator
       {
           Button b = new Button();
           b = (Button)sender;
           operandType = b.Text;
           if (operandType == "+/-" || operandType == "sqrt" ||
operandType == "1/x") //plus these unary operators
               operandCount = 1;
           else operandCount = 2;

           isFirstChar = true;
           CommaChrUsed = false;
       }

       private void btnClear_Click(object sender, EventArgs e)
       {
           txtBoxDisplay.Text = "0,";
           firstOperand = "";
           secondOperand = "";
           isFirstChar = true;
           CommaChrUsed = false;
           operandCount = 0;
       }

       private void btnEquals_Click(object sender, EventArgs e) // =
operator
       {
           if (operandCount > 1)
           {
               switch (operandType)
               {
                   case "+":
                       {
                           double result = double.Parse(firstOperand)
+ double.Parse(secondOperand);
                           txtBoxDisplay.Text = result.ToString();
                           firstOperand = txtBoxDisplay.Text;
                           secondOperand = "";
                           operandCount = 1;
                           isFirstChar = true;
                           CommaChrUsed = false;
                       }
                       break;
                   case "-":
                       {
                           double result = double.Parse(firstOperand)
- double.Parse(secondOperand);
                           txtBoxDisplay.Text = result.ToString();
                           firstOperand = txtBoxDisplay.Text;
                           secondOperand = "";
                           operandCount = 1;
                           isFirstChar = true;
                           CommaChrUsed = false;
                       }
                       break;
                   case "*":
                       {
                           double result = double.Parse(firstOperand)
* double.Parse(secondOperand);
                           txtBoxDisplay.Text = result.ToString();
                           firstOperand = txtBoxDisplay.Text;
                           secondOperand = "";
                           operandCount = 1;
                           isFirstChar = true;
                           CommaChrUsed = false;
                       }
                       break;
                   case "/":
                       {
                           double result =
double.Parse(firstOperand) / double.Parse(secondOperand);
                           txtBoxDisplay.Text = result.ToString();
                           firstOperand = txtBoxDisplay.Text;
                           secondOperand = "";
                           operandCount = 1;
                           isFirstChar = true;
                           CommaChrUsed = false;
                       }
                       break;
                   case "%":
                       {
                           double result =
(double.Parse(firstOperand) / 100) * (double.Parse(secondOperand));
                           txtBoxDisplay.Text = result.ToString();
                           firstOperand = txtBoxDisplay.Text;
                           secondOperand = "";
                           operandCount = 1;
                           isFirstChar = true;
                           CommaChrUsed = false;
                       }
                       break;
                   default:
                       {
                           MessageBox.Show("The chosen operator is
not a valid operator.", "Unknown operator");
                           btnClear_Click(sender, e);
                       }
                       break;
               }
           }
           else if (operandCount == 1)
           {
               switch (operandType)
               {
                   case "+/-":
                       {
                           double result = -
(double.Parse(firstOperand));
                           txtBoxDisplay.Text = result.ToString();
                           firstOperand = txtBoxDisplay.Text;
                           secondOperand = "";
                           isFirstChar = true;
                           CommaChrUsed = false;
                       }
                       break;
                   case "sqrt":
                       {
                           double result =
Math.Sqrt(double.Parse(firstOperand));
                           txtBoxDisplay.Text = result.ToString();
                           firstOperand = txtBoxDisplay.Text;
                           secondOperand = "";
                           isFirstChar = true;
                           CommaChrUsed = false;
                       }
                       break;
                   case "1/x":
                       {
                           double result = 1 /
double.Parse(firstOperand);
                           txtBoxDisplay.Text = result.ToString();
                           firstOperand = txtBoxDisplay.Text;
                           secondOperand = "";
                           isFirstChar = true;
                           CommaChrUsed = false;
                       }
                       break;
                   default:
                       {
                           MessageBox.Show("The chosen operator is
not a valid operator.", "Unknown operator");
                           btnClear_Click(sender, e);
                       }
                       break;
               }
           }
       }
   }
}
Alun Harford - 12 Jun 2007 02:22 GMT
> Hello all,
>
[quoted text clipped - 4 lines]
> memorybuttons to make it more of a real calculator. I look forward
> from hearing from you.

Well from a practical point of view, as long as it works you're okay!
But here are some suggestions that might make programming easier for
your next project.

> Here's the code:
>
[quoted text clipped - 13 lines]
>         private string firstOperand = "";
>         private string secondOperand = "";

You call these variables operands, but then declare them as strings. You
should define a class (or more likely an enum) called Operand and then
things might well get easier.

>         private bool isFirstChar = true;
>         private bool CommaChrUsed = false;
[quoted text clipped - 8 lines]
>         private void btnNumeral_Click(object sender, EventArgs e)
> // buttons (0-9 + comma)

Is there really a reason for this comment?
Comments in code that isn't hacky for performance reasons are usually a
sign that things have gone wrong. Your code should generally explain itself.

>         {
>             Button b = new Button();
>             b = (Button)sender;

I don't know why you're allocating a Button here.
Button b = (Button)sender;

>             if (operandCount == 0)
>             {
[quoted text clipped - 61 lines]
>                     }
>                 }

This crazy string handling is a sign that you've poorly chosen to use
strings to store your data.

>             }
>         }
[quoted text clipped - 4 lines]
>             Button b = new Button();
>             b = (Button)sender;

Button b = (Button)sender;

>             operandType = b.Text;
>             if (operandType == "+/-" || operandType == "sqrt" ||
> operandType == "1/x") //plus these unary operators
>                 operandCount = 1;
>             else operandCount = 2;

It looks like you should be using an enum for operandType.

>             isFirstChar = true;
>             CommaChrUsed = false;
[quoted text clipped - 85 lines]
>                         break;
>                 }

This looks like copy/pasted code. Usually a sign that you need to make a
new method. That way you only have one set of code to maintain.

<snip more copy/paste code>

Alun Harford
Kevin Spencer - 12 Jun 2007 12:06 GMT
> Is there really a reason for this comment?
> Comments in code that isn't hacky for performance reasons are usually a
> sign that things have gone wrong. Your code should generally explain
> itself.

I hate to disagree, but this is patently wrong. Comments are invaluable in
terms of both communicating to other developers who may work on your code,
and when you have to come back and work on the code months or years later.
Comments are not compiled, so there is *no* reason to avoid them. And I
don't know about you, but I have written some darned complex code, and over
the years, I've learned to comment MORE, not less. I don't know how many
times I've kicked myself for NOT commenting when I've had to come back and
work on my own code later.

Signature

HTH,

Kevin Spencer
Microsoft MVP

Printing Components, Email Components,
FTP Client Classes, Enhanced Data Controls, much more.
DSI PrintManager, Miradyne Component Libraries:
http://www.miradyne.net

>> Hello all,
<snip>
> Is there really a reason for this comment?
> Comments in code that isn't hacky for performance reasons are usually a
[quoted text clipped - 4 lines]
>>             Button b = new Button();
>>             b = (Button)sender;

<snip>
> <snip more copy/paste code>
>
> Alun Harford
Bob Powell [MVP] - 14 Jun 2007 18:28 GMT
>>Is there really a reason for this comment?
Comments in code that isn't hacky for performance reasons are usually a
sign that things have gone wrong. Your code should generally explain itself.

You just failed the job interview at my company big-style.

Signature

Bob Powell [MVP]
Visual C#, System.Drawing

Ramuseco Limited .NET consulting
http://www.ramuseco.com

Find great Windows Forms articles in Windows Forms Tips and Tricks
http://www.bobpowell.net/tipstricks.htm

Answer those GDI+ questions with the GDI+ FAQ
http://www.bobpowell.net/faqmain.htm

All new articles provide code in C# and VB.NET.
Subscribe to the RSS feeds provided and never miss a new article.

>> Hello all,
>>
[quoted text clipped - 240 lines]
>
> Alun Harford
Alun Harford - 15 Jun 2007 04:03 GMT
>  >>Is there really a reason for this comment?
> Comments in code that isn't hacky for performance reasons are usually a
> sign that things have gone wrong. Your code should generally explain
> itself.
>
> You just failed the job interview at my company big-style.

There are more programming jobs than there are programmers. I'll live. :-)

However, you're trying to fill one of those jobs. Perhaps you should
think more carefully about demanding that potential employees should use
your coding style before they join your company - good programmers
adjust their programming style to the corporate environment they're
working in, and everybody's style changes with time.

However, with that out of the way, I'll now try to convert you to my way
of doing things. :-)

5 years ago, I was really into comments - I was mainly using Java at the
time, and was a fan of Javadoc.

One day I did an experiment where I spent about an hour writing some
code and then I spent another hour refactoring the code to make
absolutely sure that anybody could easily understand it, without using
any comments.

I discovered, in a very personal and practical way, that well-chosen
variable and method names made Javadoc useless. And I discovered that
when I made changes to the code, I didn't have to look through to change
all the comments relating to that code (comments that are just wrong
plague 'well-commented' code).

*** IMPORTANT BIT ***
I discovered that the best way to make each line of code understandable
is to think about readability when you write the line, instead of adding
a comment after you've written the code to try to compensate for the
fact that it's unreadable.

With practice, I found that I could write easily understandable code in
only 10% more time than rushing and writing code 'normally'. Since I
didn't need to write and maintain as much documentation, I saved time,
and I found that my code was much easier to understand.

In my current 'personal' project (>100KLOC) I have 5 comments.
High-level documentation is stored elsewhere (I use paper for that) and
documentation saying what a few lines of code do is redundent if you
make sure that those lines express what you're doing in a *clear* way.

Alun Harford
Kevin Spencer - 12 Jun 2007 12:23 GMT
Hi Arjen,

There are always ways to optimize code and make it easier to read/modify. As
a first C# program, I can not find fault with it. It's a "good start." I can
provide a few suggestions.

First, almost any program never remains the same. In later incarnations,
features are added, including business logic functionality and interface. It
is important to keep them (business and UI) separate for this reason. The
business logic is the logic that processes data. The interface is the code
that presents the data to the user, takes input from the user, and passes
commands to the business "layer" from the user. So, as a first suggestion, I
would recommend separating them out.

This means that your project should actually be 2 projects. One is a Form,
and the other is a Class Library. Now, this might seem a bit much
considering that you are just doing some basic mathematical operations here,
but remember that you hope/plan to expand on it later. Some calculators, for
example, can track various values in memory. Many can perform undo and redo
operations, which also demands memory. Some will convert from one numbering
system (radix) to another. Remember that you may plan later to be able to
re-skin your UI, or host a calculator in another project. And as you've
noticed, while the numbers in the TextBox are strings, in memory they are
numbers. Your UI should treat them as strings, and your business layer
should treat them as numbers. The business layer should be able to translate
strings to numbers and vice versa.

Keeping them separate also makes the whole thing easier to work with. When
working on the Form, you're thinking only about what the business layer
wants from the user, and how to display it, as well as take input. In the
Business layer, you only have to think about data and process.

Microsoft has some great resources about good software design:
http://msdn2.microsoft.com/en-us/practices/default.aspx

My Uncle Chutney always sez, "Big things are made up of lots of little
things." As Alun pointed out, whenever you see the same block of code
repeated, or you have copied and pasted code, you're probably looking at a
good candidate for a function, or possibly a class. Breaking your project up
into smaller logical pieces will make things much easier for you over the
long haul. And breaking the pieces up into smaller pieces is the same idea
applied recursively.

Good luck!

Signature

HTH,

Kevin Spencer
Microsoft MVP

Printing Components, Email Components,
FTP Client Classes, Enhanced Data Controls, much more.
DSI PrintManager, Miradyne Component Libraries:
http://www.miradyne.net

> Hello all,
>
[quoted text clipped - 260 lines]
>    }
> }
Arjen Logghe - 12 Jun 2007 15:53 GMT
> Hi Arjen,
>
[quoted text clipped - 45 lines]
> Kevin Spencer
> Microsoft MVP

Thanks a lot for taking the time to reply. I replied to Alun but
accidentally pressed Reply Author instead of just Reply. Perhaps Alun
would be so kind as to post my reply here :).

I'll surely take all the good advice to heart.

Regards,

Arjen.
Alun Harford - 15 Jun 2007 04:16 GMT
> Thanks a lot for taking the time to reply. I replied to Alun but
> accidentally pressed Reply Author instead of just Reply. Perhaps Alun
> would be so kind as to post my reply here :).

I don't read devnull@alunharford.co.uk, but it also doesn't actually map
to /dev/null - I've found your email :-)

It's pretty late (well, early) here but I'll try to reply to it tomorrow.

Alun Harford

> On 12 jun, 03:22, Alun Harford <devn...@alunharford.co.uk> wrote:
>
[quoted text clipped - 42 lines]
>> Comments in code that isn't hacky for performance reasons are usually a
>> sign that things have gone wrong. Your code should generally explain
itself.

> I added this comment just for this post, so I didn't have to show the
> declaration of all the individual buttons and its fields in the code.
[quoted text clipped - 197 lines]
>
> Arjen.
Alun Harford - 17 Jun 2007 00:11 GMT
>> Thanks a lot for taking the time to reply. I replied to Alun but
>> accidentally pressed Reply Author instead of just Reply. Perhaps Alun
[quoted text clipped - 4 lines]
>
> It's pretty late (well, early) here but I'll try to reply to it tomorrow.

(In case anybody is eagerly awaiting my reply, I don't really have a
response to the post).

Good luck in your programming, Arjen :-)

Alun Harford
Arjen Logghe - 17 Jun 2007 00:37 GMT
> >> Thanks a lot for taking the time to reply. I replied to Alun but
> >> accidentally pressed Reply Author instead of just Reply. Perhaps Alun
[quoted text clipped - 11 lines]
>
> Alun Harford

I had set up camp at my computer waiting for you to reply :( Nah, I'll
add a few more features tomorrow and post the source :). Maybe
somebody has any other suggestions or someone may discover some errors
in the code.

Regards,

Arjen.
Arjen Logghe - 21 Jun 2007 13:18 GMT
As promised, here is the complete source :)

http://filebeam.com/a3a8d84f25cb0e3aea109b1b70134c97

I would appreciate if someone is willing to look at the source and
offer some additional suggestions. I sticked with my initial design
because changing the whole design to the offered suggestions would be
too much work (well at least for me), that's something for me to
implement in one of my next projects :). Let me know if you find any
bugs :). The only problem I've come across is that if you press Clear
and Show Log quickly, the log is being written to while you're
attempting to read it, so the program crashes after throwing an
IOException. I didn't know a way to force File.Create(logPath) to
finish before processing the btnShowLog event, other then
multithreading or asynchronous stuff. Which is something that I'm not
quite comfortable with ;).

Thanks for your time.

Regards,

Arjen.
Arjen Logghe - 21 Jun 2007 21:05 GMT
Fixed the bug :)

private void btnClearLog_Click(object sender, EventArgs e)
       {
           FileStream fs = new FileStream(logPath, FileMode.Create);
           fs.Close();
       }

Final source:

http://filebeam.com/2086b7f8ea9c57d0f3ca27c15047459d

~Arjen.
Alun Harford - 22 Jun 2007 00:01 GMT
> As promised, here is the complete source :)
>
[quoted text clipped - 5 lines]
> too much work (well at least for me), that's something for me to
> implement in one of my next projects :)

I really, really think that's something to implement in this project, so
 you can see how much less painful it is. :-)

Alun Harford

Free Magazines

Get these publications absolutely FREE for up to 12 months. There are no hidden fees and no obligation. Simply choose a title, complete the application form and submit it. Read more ...

Oracle MagazineNetwork ComputingComputer WorldBio-IT WorldeWeekInformation WeekInfosecurity
 
Sign In
Join
My Latest Posts
My Monitored Threads
My Blog
My Photo Gallery
My Profile
My Homepage

Start New Thread
Enable EMail Alerts
Rate this Thread



©2008 Advenet LLC   Privacy Policy - Terms of Use
This website includes both content owned or controlled by Advenet as well as content owned or controlled by third parties.