Make my code more clean working with methods in C#

Question

0.00/5 (No votes)

See more:

Hello first of all this is my first question ever asked here, so go easy on me.I am trying to learn C# and thinking that my passion for weightlifting I could create some automated program. In this case, calculate your 5/3/1 workout automatically. My problem is that I would like to learn how to use methods, and I think I can implement them very well here, the problem is I don't know how to attack this problem.Here is my code so far:

namespace _531Calculator
{
    class Program
    {
        static void Main(string[] args)
        {
            //Default
            Console.WindowHeight = 28;
            Console.WindowWidth = 60;
            Console.ForegroundColor = ConsoleColor.White;


            Console.Title ="5/3/1 Calculator";

            Console.WriteLine("----Your max's---- \n");

            //Bench Max
            Console.Write("Your Bench max: ");
            double benchMax = Convert.ToDouble(Console.ReadLine());

            //Bench Week 1
            double deadliftWeekOne_65 = benchMax * 0.65;
            double deadliftWeekOne_75 = benchMax * 0.75;
            double deadliftWeekOne_85 = benchMax * 0.85;

            //Deadlift Max
            Console.Write("Your Deadlift max: ");
            double deadliftMax = Convert.ToDouble(Console.ReadLine());

            //Deadlift Week 1
            double benchWeekOne_65 = deadliftMax * 0.65;
            double benchWeekOne_75 = deadliftMax * 0.75;
            double benchWeekOne_85 = deadliftMax * 0.85;

            //Squats Max
            Console.Write("Your Squat max: ");
            double squatMax = Convert.ToDouble(Console.ReadLine());

            //Squat Week 1
            double squatWeekOne_65 = squatMax * 0.65;
            double squatWeekOne_75 = squatMax * 0.75;
            double squatWeekOne_85 = squatMax * 0.85;

            //OHP Max
            Console.Write("Your OHP max: ");
            double ohpMax = Convert.ToDouble(Console.ReadLine());

            //OHP Week 1
            double ohpWeekOne_65 = ohpMax * 0.65;
            double ohpWeekOne_75 = ohpMax * 0.75;
            double ohpWeekOne_85 = ohpMax * 0.85;

            Console.WriteLine("\n ----Week 1---- \n");

            Console.WriteLine("----DAY 1----");
            Console.WriteLine("A : Bench: 3 x 5 @ " + benchWeekOne_65 + ", " + benchWeekOne_75 + ", " + benchWeekOne_85);
            Console.WriteLine("B : Dumbbell Chest Press @ 5 x 15");
            Console.WriteLine("C : Barbell Row @ 5 x 10 \n");

            Console.WriteLine("----DAY 2----");
            Console.WriteLine("A : Deadlift: 3 x 5 @ " + deadliftWeekOne_65 + ", " + deadliftWeekOne_75 + ", " + deadliftWeekOne_85);
            Console.WriteLine("B : Hamstring Curls @ 5 x 12");
            Console.WriteLine("C : Assited Work 2: Leg Raises(Abs) @ 5 x 15 \n");

            Console.WriteLine("----DAY 3----");
            Console.WriteLine("A : OHP: 3 x 5 @ " + ohpWeekOne_65 + ", " + ohpWeekOne_75 + ", " + ohpWeekOne_85);
            Console.WriteLine("B : Dip @ 5 x 15");
            Console.WriteLine("C : Chin-Up 5 x 12 \n");

            Console.WriteLine("----DAY 4----");
            Console.WriteLine("A : Squat: 3 x 5 @ " + squatWeekOne_65 + ", " + squatWeekOne_75 + ", " + squatWeekOne_85);
            Console.WriteLine("B : Stiffdead Deadlifts: @ 5 x 10 ");
            Console.WriteLine("C : Leg Press: @ 5 x 15 \n");

            Console.ReadLine(); 

        }

    }
}

What I have tried:

Well since i am here for help, and not sure how to attack this problem, any help is welcome.

Posted 18-Mar-18 20:18pm

Member 13733578

Updated 19-Mar-18 3:47am

Add a Solution

2 solutions

Solution 2

Hey,

I am unsure of where to post my updates. But this is what i cam up with, i would reayll like some feedback on how to make it better. - Thank you.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace _531Calculator
{
    class Program
    {
        static void Main(string[] args)
        {
            //Default
            Console.WindowHeight = 28;
            Console.WindowWidth = 60;
            Console.ForegroundColor = ConsoleColor.White;

            Console.Title = "5/3/1 Calculator";

            Console.WriteLine("----Your max's---- \n");

            double getDouble(string prompt)
            {
                double result;
                do
                {
                    Console.Write(prompt);                          //Calls benchMax..DeadliftMax..etc(in order)
                    string input = Console.ReadLine();              //Writes the string `Your Bench Max:`
                    if (double.TryParse(input, out result))
                    {
                        return result;
                    }
                    else
                    {
                        Console.WriteLine("Not valid", input);
                    }

                } while (true);
            }


            double benchMax = getDouble("Your Bench max: ");
            double deadliftMax = getDouble("Your Deadlift max: ");
            double squatMax = getDouble("Your Squat max: ");
            double ohpMax = getDouble("Your OHP max: ");


            //Bench Week 1
            double deadliftWeekOne_65, deadliftWeekOne_75, deadliftWeekOne_85;
            deadliftWeekOne(deadliftMax, out deadliftWeekOne_65, out deadliftWeekOne_75, out deadliftWeekOne_85);

            //Deadlift Week 1
            double benchWeekOne_65, benchWeekOne_75, benchWeekOne_85;
            benchWeekOne(benchMax, out benchWeekOne_65, out benchWeekOne_75, out benchWeekOne_85);

            //Deadlift Week 1
            double squatWeekOne_65, squatWeekOne_75, squatWeekOne_85;
            squatWeekOne(squatMax, out squatWeekOne_65, out squatWeekOne_75, out squatWeekOne_85);

            //OHP Week 1
            double ohpWeekOne_65, ohpWeekOne_75, ohpWeekOne_85;
            ophWeekOne(ohpMax, out ohpWeekOne_65, out ohpWeekOne_75, out ohpWeekOne_85);

            Console.WriteLine("\n ----Week 1---- \n");

            Console.WriteLine("----DAY 1----");
            Console.WriteLine("A : Bench: 3 x 5 @ " + benchWeekOne_65 + ", " + benchWeekOne_75 + ", " + benchWeekOne_85);
            Console.WriteLine("B : Dumbbell Chest Press @ 5 x 15");
            Console.WriteLine("C : Barbell Row @ 5 x 10 \n");

            Console.WriteLine("----DAY 2----");
            Console.WriteLine("A : Deadlift: 3 x 5 @ " + deadliftWeekOne_65 + ", " + deadliftWeekOne_75 + ", " + deadliftWeekOne_85);
            Console.WriteLine("B : Hamstring Curls @ 5 x 12");
            Console.WriteLine("C : Assited Work 2: Leg Raises(Abs) @ 5 x 15 \n");

            Console.WriteLine("----DAY 3----");
            Console.WriteLine("A : OHP: 3 x 5 @ " + ohpWeekOne_65 + ", " + ohpWeekOne_75 + ", " + ohpWeekOne_85);
            Console.WriteLine("B : Dip @ 5 x 15");
            Console.WriteLine("C : Chin-Up 5 x 12 \n");

            Console.WriteLine("----DAY 4----");
            Console.WriteLine("A : Squat: 3 x 5 @ " + squatWeekOne_65 + ", " + squatWeekOne_75 + ", " + squatWeekOne_85);
            Console.WriteLine("B : Stiffdead Deadlifts: @ 5 x 10 ");
            Console.WriteLine("C : Leg Press: @ 5 x 15 \n");

            Console.ReadLine();

        }

        private static void ophWeekOne(double ohpMax, out double ohpWeekOne_65, out double ohpWeekOne_75, out double ohpWeekOne_85)
        {
            ohpWeekOne_65 = ohpMax * 0.65;
            ohpWeekOne_75 = ohpMax * 0.75;
            ohpWeekOne_85 = ohpMax * 0.85;
        }

        private static void squatWeekOne(double squatMax, out double squatWeekOne_65, out double squatWeekOne_75, out double squatWeekOne_85)
        {
            squatWeekOne_65 = squatMax * 0.65;
            squatWeekOne_75 = squatMax * 0.75;
            squatWeekOne_85 = squatMax * 0.85;
        }

        private static void benchWeekOne(double benchMax, out double benchWeekOne_65, out double benchWeekOne_75, out double benchWeekOne_85)
        {
            benchWeekOne_65 = benchMax * 0.65;
            benchWeekOne_75 = benchMax * 0.75;
            benchWeekOne_85 = benchMax * 0.85;
        }

        private static void deadliftWeekOne(double deadliftMax, out double deadliftWeekOne_65, out double deadliftWeekOne_75, out double deadliftWeekOne_85)
        {
            deadliftWeekOne_65 = deadliftMax * 0.65;
            deadliftWeekOne_75 = deadliftMax * 0.75;
            deadliftWeekOne_85 = deadliftMax * 0.85;
        }
    }
}

Posted 19-Mar-18 3:48am

Member 13733578

Comments

Richard Deeming 19-Mar-18 11:06am

The ophWeekOne, squatWeekOne, benchWeekOne and deadliftWeekOne are all doing exactly the same thing. You could replace them with a single method:

private static void CalculateWeek1(double max, out double value65, out double value75, out double value85)
{
    value65 = max * .65;
    value75 = max * .75;
    value85 = max * .85;
}

Usage:

// Bench Week 1
double benchWeekOne_65, benchWeekOne_75, benchWeekOne_85;
CalculateWeekOne(benchMax, out benchWeekOne_65, out benchWeekOne_75, out benchWeekOne_85);

// Deadlift Week 1
double deadliftWeekOne_65, deadliftWeekOne_75, deadliftWeekOne_85;
CalculateWeekOne(deadliftMax, out deadliftWeekOne_65, out deadliftWeekOne_75, out deadliftWeekOne_85);

// Squat Week 1
double squatWeekOne_65, squatWeekOne_75, squatWeekOne_85;
CalculateWeekOne(squatMax, out squatWeekOne_65, out squatWeekOne_75, out squatWeekOne_85);

// OHP Week 1
double ohpWeekOne_65, ohpWeekOne_75, ohpWeekOne_85;
CalculateWeekOne(ohpMax, out ohpWeekOne_65, out ohpWeekOne_75, out ohpWeekOne_85);

You could also clean up the Console output slightly by using Composite Formatting[^]:

Console.WriteLine("A : Bench: 3 x 5 @ {0}, {1}, {2}", benchWeekOne_65, benchWeekOne_75, benchWeekOne_85);

Or Interpolated Strings[^]

Console.WriteLine($"A : Bench: 3 x 5 @ {benchWeekOne_65}, {benchWeekOne_75}, {benchWeekOne_85}");

OriginalGriff 19-Mar-18 11:18am

Start by dumping the comments.The first one you added to getDouble is wrong - and wrong comments are a really bad idea! The second one is also wrong, and more wrong than teh first one!
Comments are there to enhance understanding - so comment what things do if it isn't clear and self documented. So don't comment
Console.WriteLine(prompt);
because it's obvious from reading it what it does - it prints the "give me this data prompt" to the user.
By all means comment the method and it's parameters, but don't comment stuff that isn't obvious. Commenting "prompt" to explain it's the string that tells the user what is expected of him - that's fine and a damn good idea. (And very easy to do: put your cursor in the blank line above the function declaration line, and press "/" three times. VS will automatically insert an intelligent template for you with the parameters set out ready for descriptions - just add it between the ">" and "<" characters and Intellisense will pick it up for you as if it was a .NET framework method!)

Oh - and change the method name back: there are rules for naming conventions and methods do not start with a lowercase character in C#

Your methods at the bottom of that ... don't they look rather "samey" to you? They do to me...

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

OriginalGriff · Accepted Answer · 2018-03-18T21:09:00

Solution 1

First off, don't use Convert methods: users mistype the input all the time, and the Convert methods will make you app crash if they do that. Use the double.TryParse methods instead.

In fact, that's a good intro to using a method.
At the moment, you do the same thing a number of times:

            Console.Write("Your Bench max: ");
            double benchMax = Convert.ToDouble(Console.ReadLine());
...
            Console.Write("Your Deadlift max: ");
            double deadliftMax = Convert.ToDouble(Console.ReadLine());
...
            Console.Write("Your Squat max: ");
            double squatMax = Convert.ToDouble(Console.ReadLine());
...
            Console.Write("Your OHP max: ");
            double ohpMax = Convert.ToDouble(Console.ReadLine());

And convert those to use TryParse would make your code fairly unwieldy.
So instead, let's move the code in to a method instead. Start by writing a method - it needs to return a double, and it needs to take a string prompt as an input parameter. So it's signature is simple:

double GetDouble(string prompt)

And it's content isn't complicated either:
1) Print the prompt.
2) Get the user input
3) Convert it to a double
4) If the conversion failed, tell the user there is a problem and go back to (1)
5) Return the value.
So let's write it:

double GetDouble(string prompt)
    {
    double result;
    do
        {
        Console.Write(prompt);
        string inp = Console.ReadLine();
        if (double.TryParse(inp, out result))
            {
            return result;
            }
        Console.WriteLine("\"{0}\" is not a valid double value!", inp);
        } while(true);
    }

All we have to do now is use the new method:

            double benchMax = GetDouble("Your Bench max: ");
...
            double deadliftMax = GetDouble("Your Deadlift max: ");
...
            double squatMax = GetDouble("Your Squat max: ");
...
            double ohpMax = GetDouble("Your OHP max: ");

See how much cleaner that bit of code is now?

Now look though your code, and see if you can find anything else which could benefit from the same attention!

Posted 18-Mar-18 21:09pm

OriginalGriff

Comments

Member 13733578 19-Mar-18 3:39am

Hello Thank you for your detaild answer. I don't understand the TryParse part, could you elaborate please. Like how and where to use it. Do i only need to use it in the method ?

OriginalGriff 19-Mar-18 3:52am

You can use TryParse anywhere you want, not just "in a method".
It does the same thing as Convert.ToDouble, but it returns a bool to indicate success / fail instead of throwing an exception.

So if the user enters "hello.6" instead of "12345.6" you can gracefully tell them what was wrong instead of your app just crashing. If you sit there and type in a dozen numbers, then the app crashes because you typed two '.'s instead of one you wouldn't be a happy bunny! :laugh:

Member 13733578 19-Mar-18 3:59am

Hello, Thank you again. I used your method and it acctualy works really well. I still dont really understand what it does. But i guess it comes with time. Thank you.

OriginalGriff 19-Mar-18 4:30am

What part don't you understand - if you explain what you can cope with, I'll try and "fill in the blanks". But I can't see you or your screen, so you need to be fairly explicit about your comprehension! :laugh:

Don't worry about "feeling silly" or "looking like an idiot" - you won't: we understand beginners, and that they do get confused. We don't try to make you feel stupid, honest!

Member 13733578 19-Mar-18 9:47am

The if part is really in the clouds for me.

OriginalGriff 19-Mar-18 11:09am

OK: you know what an "if" does, but a quick recap:
if (a)
b;
else
c;

If "a" evaluates to true, then "b" is executed, otherwise "c" is.
"a" can be anything which ends up as a bool:

i > 16
myList.Count == 0

In this case, "a" is a call to a method which returns a bool value: double.TryParse.
So if TryParse returns true, then the function exits, and returns the converted value. If it returns false, then the loop continues with t6he error message print.
That should all be pretty clear to you, I hope?
The condition "a" in this case is a little complicated, but I'm deliberately going to "gloss over" the details as they will come later in your course.
"double" is a class (gloss, gloss) which has a static (gloss, gloss) method called TryParse which takes two parameters.
The first is a string, and it's the data the user entered that you want to convert. The second is a "special" parameter - hence the "out" in front of it (gloss, gloss, gloss, gloss, gloss) which says "the method you call will change the value of this variable". If you think about it, TryParse needs to return two bits of information: the converted value and a flag which says "it converted OK" or "that wasn't a valid number" and you can't return two values from one method (gloss, gloss, gloss, GLOSS) so you need a mechanism to get the second value back to your code.

That's all there is! Does that make sense?

F-ES Sitecore 19-Mar-18 5:45am

TryParse is just a different way of converting the string to a number. Convert assumes that the string you are trying to convert can be converted to the desired type. So if converting to int strings like "123", "0" and so on are ok, but if you give it a string that it can't convert like "abc" then it throws an error and your program stops. TryParse is more robust, if the string can't be converted it simply returns false allowing your code to continue so you can show the error. So Convert assumes you have done something that validates the text before using it, where TryParse does the validating for you.

Think of Convert being someone going to the gym and telling a personal trainer that they're going to bench 200lbs, well Convert just loads the bar up and let's then bench and if they're built like a twig they get crushed by the bar. Whereas TryParse looks at the person and considers if they're strong enough to bench that weight and if they're not just says "No" rather than letting the person get crushed.