Absolute Values and Fractions

Question

$$ \frac{|x|-|y|}{1+|xy|} $$

How can I reduce the amount of code without losing quality? What are your tips? Your comments on the structure, logic, in general, everything. How to make the type of data entered by the user determined automatically?

 using System;
 class taskTwo {

static void Main() {

    int x, y; // Declare variables
    double z, w;
    Console.WriteLine("Please, enter two the numbers");
   x = int.Parse(Console.ReadLine()); //The user enters
    y = int.Parse(Console.ReadLine());

    if (x == 0 && y == 0)//If two zeros are entered, then the numerator is 
                                                            zero. 
    {
        Console.WriteLine("The numerator and denominator can not be zero");
    }

    else {
        z = Math.Abs(x) - Math.Abs(y);//Know what the numerator is equal to 
        if (z == 0)//If the numerator is zero, then ... 
              {
                    Console.WriteLine("Numerator is zero, division is impossible");
                }
        else {
            w = z / (1 + Math.Abs(x) * Math.Abs(y)); //Final calculation
            Console.WriteLine("W = " + w); //Display the result on the screen
        }
    }


}}

Answer 1

How can I reduce the amount of code without losing quality?

Well... there is no quality yet ;-) and actually you are going to gain some code if you want to improve it.

I won't try to prove whether there are numbers that can break this fraction in any way but for the sake of the review let's assume the denominator can be zero.

---

The first improvement you can make is to create a class and a method for the formula because in a real application you wan't to write some test before you let anyone use it so how about a Calculator? I don't know if this formula has any special name so I just named it Fraction1.

Apart from putting it in a testable calculator you should work on the names. What is z or w? How are those letters related to the formula? If you are splitting it into multiple variables then give them resonable names.

Should it happen that the denominator is 0 then the DivideByZeroException is to be thrown and handeled.

class Calculator
{
    public double CalcFraction1(double x, double y)
    {
        var numerator = Math.Abs(x) - Math.Abs(y);
        var denominator = (1 + Math.Abs(x) * Math.Abs(y));

        if (denominator == 0)
        {
            throw new DivideByZeroException();
        }

        return numerator / denominator;
    }
}

You now have a calculator that works. The next step is to read from the console... but maybe you want to read the numbers until the input is correct? Currently it would crash if you enter a letter. So create a helper for it that will try to parse the input until it's a valid value. Instead of using int for this job you could use double to get more precision:

private static double ReadNumber()
{
    double value;
    while (!double.TryParse(Console.ReadLine(), out value)) { }
    return value;
}

The last step is to put everything together, this is, create the calculator, read the the numbers and calculate the result or handle the exception if it wasn't possible:

void Main()
{
    var calculator = new Calculator();

    Console.WriteLine("Please, enter two numbers");

    var x = ReadNumber();
    var y = ReadNumber();

    try
    {
        Console.WriteLine($"W = {calculator.CalcFraction1(x, y)}");
    }
    catch (DivideByZeroException)
    {
        Console.WriteLine("Cannot calculate fraction1 because the denominator is zero.");
    }
}

You should always try to encapsulate pieces of code that should be tested or can work and be reused independently for example you could copy/paste the ReadNumber method and use it for a different console application or tune it and add additional output about an invalid number without touching any other part. This is what we always try to achieve. Small units that do their job well and can be use like pieces of puzzle to build something bigger, something useful.

Answer 2

There is no integer value for which this undefined. Having a zero in the numerator is a legal math operation that results as zero. having a zero in the denominator would result in an undefined result, but that concern is mitigated by the fact that the absolute value of the product of two integers will be greater than or equal to zero and then you add one to that product before doing the division. In other words, the smallest denominator that the equation could have is one which is legal. So you could remove the if statements

using System;

class TaskTwo {

    static void Main() {
        // Declare variables
        int x, y; 
        double w;

        Console.WriteLine("Please, enter two the numbers");
        x = int.Parse(Console.ReadLine());
        y = int.Parse(Console.ReadLine());


        w=(Math.Abs(x) - Math.Abs(y))/ (1 + Math.Abs(x*Y));

        // Display the result on the screen
        Console.WriteLine("W = " + w);
       }
    }

This should suffice.

Answer 3

You really should improve your code formatting. Code which is formatted well (indentation, concise brackets) is easier to read and understand. I'd also suggest you to improve your variable naming a bit. For example z isn't too descriptive. Instead I'd name it numerator and name w result.

You don't need to declare your variables at the top of your method. You rather should declare them closer to the usage.

Your comments describe what you are doing. It's not what you should be doing. Comments should rather describe why are you doing what you are doing.

Compare my below final formatting to yours

using System;

class TaskTwo {

    static void Main() {

        Console.WriteLine("Please, enter two the numbers");
        int x = int.Parse(Console.ReadLine());
        int y = int.Parse(Console.ReadLine());

        // If two zeros are entered, then the numerator is zero 
        if (x == 0 && y == 0)
        {
            Console.WriteLine("The numerator and denominator can not be zero");
        }
        else 
        {
            // Know what the numerator is equal to
            double nominator = Math.Abs(x) - Math.Abs(y);

            //If the numerator is zero, then ... 
            if (z == 0)
            {
                Console.WriteLine("Numerator is zero, division is impossible");
            }
            else 
            {
                // Final calculation
                double result = nominator / (1 + Math.Abs(x) * Math.Abs(y)); 
                // Display the result on the screen
                Console.WriteLine("Result = " + result);
            }
        }
    }   
}

Answer 4

• Math.Abs(x) can throw an exception. If x == int.MinValue, Math.Abs refuses to take its absolute value. Same thing for y, of course. Those inputs could be rejected, but it's probably nicer to just do the right thing. This could be fixed by converting the inputs to long, or to double, or by implementing an unsigned abs. Some of the options make the second part harder.
• Math.Abs(x) * Math.Abs(y) can easily overflow, in fact it can result in -1 (try x = 3, y = 0x55555555) and then there would be a division by zero. Taking the product modulo 2³² (and then reinterpreted as signed) does not look intended here. Converting the inputs to a wider type mostly fixes this, there is some precision loss with double though.

So with conversion to double, especially useful if you actually wanted a floating point division all along:

using System;

class TaskTwo
{
    static void Main()
    {
        Console.WriteLine("Please, enter two the numbers");

        // take ints as input but immediately convert them to doubles
        double x = int.Parse(Console.ReadLine());
        double y = int.Parse(Console.ReadLine());

        double w = (Math.Abs(x) - Math.Abs(y)) / (1 + Math.Abs(x*y));

        Console.WriteLine("W = " + w);
    }
}

Answer 5

Reduce amount of code should not be your objective.
You have some code problems.

That code will perform integer math. Will not get anything after the decimal.

Exclude 0 in the numerator is point less.

It fails to validate input.

I would recommend decimal over double here for edge accuracy cases.

It seems silly to allow negative integer input if you are going to take the absolute value but that is what you are doing.

This should take care of validation and perform double math

class Program
{
    static void Main(string[] args)
    {
        int i = ReadInt();
        int j = ReadInt();
        //works at extremes
        //i = int.MaxValue;
        //j = int.MinValue;
        double a = Math.Abs((double)i);
        double b = Math.Abs((double)j);
        double w = (a - b) / (1D + (a * b));
        Console.WriteLine("answer is " + w.ToString());
    }
    static int ReadInt()
    {
        int readInt;
        bool b = false;
        string s = string.Empty;
        do
        {
            Console.WriteLine("Enter integer");
            s = Console.ReadLine();
            b = int.TryParse(s, out readInt);
            if(!b)
            {
                Console.WriteLine("Not a valid integer");
            }
        } while (!b);
        return readInt;
    }
}