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

Calculating distance and angle between two points, C# program

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

Problem

I like to think I'm an intermediate programmer, but I learned to code with the Unity engine so I'm still getting used to not using it, and I've never had someone review my code before. I don't know any other programmers, most people in my town don't even have a computer. I'd appreciate it greatly if you'd be so kind and tell me what you think of my little script here. Rather, be brutal if you want.

My script works fine, but I have some concerns, like the readability of calling a Boolean method with a bunch of parameters in the condition of a while loop for example.

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

namespace Course_Assignment
{
class DistanceCalculator
{
///
/// This class calculates the distance between two points and the angle of the slope.
///

static void Main(string[] args)
{
float point1X = 0;
float point1Y = 0;
float point2X = 0;
float point2Y = 0;

float deltaX = 0;
float deltaY = 0;

double distance = 0;
double angle;

Console.WriteLine("Welcome. This program calculates the distance \nbetween two points as well as their angle.\nPlease type the prompted values.");

//GetPoints() defines all the point coordinates via user input and
//returns true if the data entered was invalid.
NewPoints : while (GetPoints(ref point1X, ref point1Y, ref point2X, ref point2Y))
{
Console.WriteLine("Invalid data has been entered. Try again.");
}

//Getting the change in x and y from point 1 to 2.
deltaX = point2X - point1X;
deltaY = point2Y - point1Y;

//Calc() uses the Pythagorean Theorem to calculate and return
//distance, and calculates angle in degrees by multiplying the
//tangent of deltaX and deltaY.
distance = Calc(deltaX, deltaY, out angle);

//Displaying calculation

Solution


  • You should avoid using goto statements. Not because it took me a minute to recall what NewPoints : syntax even means, but because goto makes it extremely hard to follow the execution flow. Cases where goto makes code more readable are rare, and yours is not on of those. Just use another while (or do while) loop instead.



-
Calc is a poor name for a method, because it doesn't say anything about what sort of calculation it performs. It calculates distance you say? Name it CalcDistance then. Oh, wait, it also calculates angle! Maybe CalcDistanceAndAngle then? That doesn't look right... Which is an indication, that you probably need two separate methods: one to calculate angle and one to calculate distance (related to SRP).

var distance = CalcDistance(...);
var angle = CalcAngle(...);


-
There is a lot of copy-pasting in GetPoints method. You should probably have a method to query single value. For example

static float RequestValue(string message)
{ 
    float value;
    while (true) 
    {
        Console.WriteLine(message);
        Console.SetCursorPosition(14, Console.CursorTop - 1);
        if (float.TryParse(Console.ReadLine(), out value)) break;
        Console.WriteLine("Invalid data has been entered. Try again."); 
    } 
    return value;
}


Then you can just initialize individual points:

var point1X = RequestValue("\nPoint 1\nX Coordinate: ");


-
I am pretty sure you don't need to move cursor, if you call Console.Write instead of Console.WriteLine.

-
You are using both floats and doubles in your calculations. You should probably use one or the other.

-
else return; is unnecessary.

-
I think it would make more sense for Calc to take actual points as input instead of "delta"-s. Meaning you should move:

//Getting the change in x and y from point 1 to 2.
deltaX = point2X - point1X;
deltaY = point2Y - point1Y;


inside Calc's body.

Code Snippets

var distance = CalcDistance(...);
var angle = CalcAngle(...);
static float RequestValue(string message)
{ 
    float value;
    while (true) 
    {
        Console.WriteLine(message);
        Console.SetCursorPosition(14, Console.CursorTop - 1);
        if (float.TryParse(Console.ReadLine(), out value)) break;
        Console.WriteLine("Invalid data has been entered. Try again."); 
    } 
    return value;
}
var point1X = RequestValue("\nPoint 1\nX Coordinate: ");
//Getting the change in x and y from point 1 to 2.
deltaX = point2X - point1X;
deltaY = point2Y - point1Y;

Context

StackExchange Code Review Q#154638, answer score: 7

Revisions (0)

No revisions yet.