Skip to main content
replaced http://programmers.stackexchange.com/ with https://softwareengineering.stackexchange.com/
Source Link

Read about the using of region's here: are-regions-an-antipattern-or-code-smell.

Read about the using of region's here: are-regions-an-antipattern-or-code-smell.

Source Link
Heslacher
  • 50.4k
  • 5
  • 81
  • 175

Naming

Names of classes, methods and variables should be as descriptive and meaningful as possible. Assume Sam the Maintainer has to fix a bug/unexpected behaviour in your code and sees classes named Assignment5 and variables named fiNum, strRat, ... Sam the Maintainer would think wtf is going on in this code.

Grasping the context of the code at first glance should be the top priority while writing code.

Speaking about Assignment5, if you would have named the class Fraction or Rational this and some more comments would have been superfluous

//adding the two fractions together 
public static Assignment5 operator +(Assignment5 rat1, Assignment5 rat2)

Comments

Comments should describe why something is done in the way it is done. Let the code speak for itself about what is done by using meaningful names.

Comments like

//blank form1 Load
//calls constructor
//Subtract button code  

doesn't add any value to the code. This is just noise which should be removed.


Style

If you have choosen a style to code something e.g if..else you should stick to that style. Right now you are mixing different styles for the same thing.

if (fiDenom == 0)
    fbValid = false;
else
    fbValid = true; 

sometimes you place the statement after the if and else on a new line,

if (rat2 == 0) return rat1;
else
    return gcd(rat2, rat1 % rat2);  

sometimes you place it on the same line as the if and on a new line for the else,

if (rat1.fbValid && rat2.fbValid)
{
    rat1.fiNum = rat1.fiNum * rat2.fiDenom + rat1.fiDenom * rat2.fiNum;
    rat1.fiDenom = rat1.fiDenom * rat2.fiDenom;
}
else rat1.fbValid = false;  

and sometimes you place it on the same line as the else.

Much better IMHO would be to always use braces {} to make your code less error prone. IMHO it is also easier to read, because it is always the same style.


Declaring multiple variables on the same line reduces readability. You should always try to improve the readability of the code. So instead of

Assignment5 ratNum1, ratNum2, ratSum;  
ratNum1 = new Assignment5(txtRat1.Text);  //calls constructor         
ratNum2 = new Assignment5(txtRat2.Text); 
ratSum = new Assignment5("0/1");

you should use

Assignment5 ratNum1 = new Assignment5(txtRat1.Text);
Assignment5 ratNum2 = new Assignment5(txtRat2.Text);
Assignment5 ratSum = new Assignment5("0/1");  

Read about the using of region's here: are-regions-an-antipattern-or-code-smell.