3
\$\begingroup\$

I'm doing a simple calc for my school. The code looks okay to me, but I'm not sure if there is something to improve, but probably there is always something to improve. Any ideas how to make it look and work better?

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

namespace kalkulacka
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine("Daniel Nosek");
            Console.WriteLine("Výpočet obvodu a obsahu - trojúhelník, pravidelný šestiúhelník");
            Console.WriteLine("Zvolte si obrazec:");
            Console.WriteLine("1 - trojúhelník");
            Console.WriteLine("2 - pravidelný šestiúhelník");

            int VolbaObrazce = int.Parse(Console.ReadLine());
            double obvod = 0;
            double obsah = 0;

            bool prepocitat = false;

            do
            {
                Console.Clear();

                switch (VolbaObrazce)
                {
                    case 1:
                        double a = PrectiPromennou("Zadejte délku strany a:");
                        double b = PrectiPromennou("Zadejte délku strany b:");
                        double c = PrectiPromennou("Zadejte délku strany c:");

                        obvod = ObvodTrojuhelniku(a, b, c);
                        obsah = ObsahTrojuhelniku(a, b, c);
                        break;

                    case 2:
                        double d = PrectiPromennou("Zadejte délku strany d:");

                        obvod = ObvodSestiuhelniku(d);
                        obsah = ObsahSestiuhelniku(d);
                        break;
                    default:
                        {
                            Console.WriteLine("Neplatná volba");
                        }
                        return;
                }

                // vysledky, zaokrouhleny na dve desetinna mista
                Console.WriteLine("obvod: " + Math.Round(obvod, 2));
                Console.WriteLine("obsah: " + Math.Round(obsah, 2));

                /* loop pro pripad kdy uzivatel chce vypocet znova s jinymi hodnotami
                   promenna recalculate musi byt rovna 1, jinak se program vypne */
                prepocitat = PrectiPromennou("Pro výpočet s jinými rozměry stiskněte 1:") == 1;
            }
            while (prepocitat);
        }

        /*Puvodne jsem pouzival pri cases
        Console.WriteLine("Zadejte délku strany x:");
        double x = double.Parse(Console.ReadLine());
        nicmene resit to takhle mi prijde jednodussi.*/
        static double PrectiPromennou(string text)
        {
            Console.Write(text);
            return double.Parse(Console.ReadLine());
        }

        /// <summary>
        /// Vypocet obvodu pomoci souctu stran
        /// </summary>
        /// <param name="a">delka strany a</param>
        /// <param name="b">delka strany b</param>
        /// <param name="c">delka strany c</param>
        /// <returns>obvod trojuhelniku</returns>
        static double ObvodTrojuhelniku(double a, double b, double c)
        {
            return a + b + c;
        }

        /// <summary>
        /// Vypocet obsahu pomoci heronova vzorce
        /// </summary>
        /// <param name="a">delka strany a</param>
        /// <param name="b">delka strany b</param>
        /// <param name="c">delka strany c</param>
        /// <returns>obsah trojuhelniku</returns>               
        static double ObsahTrojuhelniku(double a, double b, double c)
        {
            double s = (a + b + c) / 2;
            return Math.Sqrt(s * (s - a) * (s - b) * (s - c));
        }

        /// <summary>
        /// Vypocet obvodu pomoci soucinu stran
        /// </summary>
        /// <param name="d">delka strany d</param>
        /// <returns>obvod sestiuhelniku</returns>
        static double ObvodSestiuhelniku(double d)
        {
            return 6 * d;
        }

        /// <summary>
        /// vypocet obsahu 
        /// </summary>
        /// <param name="d">delka strany d</param>
        /// <returns>obsah sestiuhelniku</returns>
        static double ObsahSestiuhelniku(double d)
        {
            return ((3 * Math.Sqrt(3) * Math.Pow(d, 2))) / 2;
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ what .net framework are you using? 4.6? 4.7? lower? \$\endgroup\$ – Patrick Artner Dec 16 '18 at 10:02
2
\$\begingroup\$

Depending on your style and .net version you can change some of your methods to Expression bodied function members:

/// documentation
static double ObvodTrojuhelniku(double a, double b, double c) => a + b + c

/// documentation
static double ObvodSestiuhelniku(double d) => 6 * d

/// documentation
static double ObsahSestiuhelniku(double d) => ((3 * Math.Sqrt(3) * Math.Pow(d, 2))) / 2;

I really like those small function you are using, they are easily testable and follow SRP - they have only one function and one reasons to change - if at all.


You should add input validation - if a user inputs "QWER" and you int.Parse(..) it directly, it will crash. Similarly for double.Parse(...)

Consider using something along the lines of:

public static class InputHelper
{
    /// <summary>
    /// Ask unser for input until a valid double is parsed
    /// </summary>
    /// <param name="text">The text presented to the user before inputting.</param>
    /// <returns>double</returns>
    public static double GetDouble(string text)
    {
        string p = null;
        while (true)
        {
            Console.WriteLine(text);
            try
            {
                p = Console.ReadLine();
                return double.Parse(p);
            }
            catch (Exception ex)
            {
                if (ex is FormatException)
                {
                    Console.WriteLine($"Invalid input: '{p}' - only numbers and 1 decimal divider allowed!");
                }
                else if (ex is OverflowException)
                {
                    Console.WriteLine($"Invalid input: '{p}' - is too big. Max: {double.MaxValue}");
                }
            }
        }
    }

    /// <summary>
    /// Ask unser for input until a valid integer is parsed
    /// </summary>
    /// <param name="text">The text presented to the user before inputting.</param>
    /// <returns>integer</returns>
    public static int GetInt(string text)
    {
        string p = null;
        while (true)
        {
            Console.WriteLine(text);
            try
            {
                p = Console.ReadLine();
                return int.Parse(p);
            }
            catch (Exception ex)
            {
                if (ex is FormatException)
                {
                    Console.WriteLine($"Invalid input: '{p}' - only numbers allowed!");
                }
                else if (ex is OverflowException)
                {
                    Console.WriteLine($"Invalid input: '{p}' - is too big. Max: {int.MaxValue}");
                }
            }
        }
    }
}

where you do int VolbaObrazce = int.Parse(Console.ReadLine()); and inside static double PrectiPromennou(string text).


If you know about templating, you can reduce the duplicate code (DRY) in your static helpers. I put them into an extra class to be used in other projects as well.

\$\endgroup\$
0
\$\begingroup\$

Handle possible exceptions across your code. Handle Exceptions

Handle parsing values directly to avoid exceptions (use TryParse instead of Parse).

int VolbaObrazce;
bool validInput= int.TryParse(Console.ReadLine(),out VolbaObrazce);
//here you shall have value in VolbaObrazce if you only provide valid int.

you have also multiple methods that handles only arithmetic operation which will be very hard to maintain.

\$\endgroup\$
  • 1
    \$\begingroup\$ int.TryParse(...) is generally used like this: if (int.TryParse(Console.ReadLine(),out var VolbaObrazce)) { .... } - you loose the reason of the "not parsable" with it. Multiple methods that do one thing very well are not hard to maintain - you can easily test them and they have very few reasons to change. \$\endgroup\$ – Patrick Artner Dec 16 '18 at 14:03
  • \$\begingroup\$ i second you in defining one responsibility per method but actually i see it so un useful to perform only one arithmetic operation per method, may be defining a property here will fit better for readability & maintainability. \$\endgroup\$ – Michel Hanna Dec 16 '18 at 22:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.