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.