HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Calculator for area and perimeter of a window

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
perimeterforandwindowcalculatorarea

Problem

This program calculates wood length and area of glass required for the window.
I am a beginner in C# and I want to know how professional programmers will write program like this.

double width, height, woodLength, glassArea;
        const double MAX_WIDTH = 5.0;
        const double MIN_WIDTH = 0.5;
        const double MAX_HEIGHT = 3.0;
        const double MIN_Height = 0.75;
        string widthString, heightString;
        Console.WriteLine("please enter width of the window");
       widthString = Console.ReadLine();
        width = Convert.ToDouble(widthString);
        if (width MAX_WIDTH)
        {
            Console.WriteLine("you entered value grater than MaX width\n" + "using maxuimum one");

            width = MAX_WIDTH;
        }
        Console.WriteLine("please enter your height of the window");
        heightString = Console.ReadLine();
        height = double.Parse(heightString);
        if (height MAX_HEIGHT) 
        {
            Console.WriteLine("you enter value grater than Max height\n" + "using maxuimum one");
            height = MAX_HEIGHT;
        }
        woodLength = 2 * (width+height);
        glassArea =  (width * height);
        Console.WriteLine("the lenth of the wood is "+woodLength+"feet");
        Console.WriteLine("the area of the glass is " + glassArea +"m^2");

Solution

Typos / Spelling issue

Attention to detail is important. Spelling errors, in variable names + strings should be avoided. They give a bad impression. You have several. mininium should be minimum, maxuimum should be maximum, Sentences should start with a capital letter etc.

Be clear with the user

You ask the user for the width/height of the window, however you don't tell the user what unit the measurements are in. Your original code you were converting to feet (by multiplying by 3.25). If you're going to do something like this, consider defining a constant with a name that reflects what it is, FeetPerMetre for example (Constants should be in Pascal, not ALL_CAPS naming). You also haven't told the user the min/max values either when you prompt for input, or when you decide to use them.

Contain your functionality

The code you've posted is all in a single block. It doesn't appear to have been wrapped in a function, suggesting that it's just sitting in a static main somewhere. Think about what elements the code contains and break it out into functions that you might be able to reuse. Some possible functions might be:

  • int GetLengthFromUser(string prompt, double minLength, double maxLength)



  • double CalculateWindowFrameLength(double width, double length)



  • double CalculateWindowPaneArea(double width, double length)



  • double ConvertFeetToMetres(double length)



Interpolated Strings

Depending on the C# version you're using, you may be able to use interpolated strings which can things like WriteLine statements look a bit cleaner:

Console.WriteLine($"The lenth of the wood is {woodLength} feet");


double.Parse / Convert.ToDouble

Both methods can fail and throw an exception if the user enters something other than a number (which will inevitably happen eventually). Consider using TryParse and re-prompting the user when it fails.

Code Snippets

Console.WriteLine($"The lenth of the wood is {woodLength} feet");

Context

StackExchange Code Review Q#149470, answer score: 5

Revisions (0)

No revisions yet.