views:

574

answers:

11

Hi,

I've completed a OOP course assignment where I design and code a Complex Number class. For extra credit, I can do the following:

  1. Add two complex numbers. The function will take one complex number object as a parameter and return a complex number object. When adding two complex numbers, the real part of the calling object is added to the real part of the complex number object passed as a parameter, and the imaginary part of the calling object is added to the imaginary part of the complex number object passed as a parameter.

  2. Subtract two complex numbers. The function will take one complex number object as a parameter and return a complex number object. When subtracting two complex numbers, the real part of the complex number object passed as a parameter is subtracted from the real part of the calling object, and the imaginary part of the complex number object passed as a parameter is subtracted from the imaginary part of the calling object.

I have coded this up, and I used the this keyword to denote the current instance of the class, the code for my add method is below, and my subtract method looks similar:

 public ComplexNumber Add(ComplexNumber c)
{
    double realPartAdder = c.GetRealPart();
    double complexPartAdder = c.GetComplexPart();

    double realPartCaller = this.GetRealPart();
    double complexPartCaller = this.GetComplexPart();

    double finalRealPart = realPartCaller + realPartAdder;
    double finalComplexPart = complexPartCaller + complexPartAdder;

    ComplexNumber summedComplex = new ComplexNumber(finalRealPart, finalComplexPart);

    return summedComplex;
}

My question is: Did I do this correctly and with good style? (using the this keyword)?

+18  A: 

The use of the this keyword can be discussed, but it usually boils down to personal taste. In this case, while being redundant from a technical point of view, I personally think it adds clarity, so I would use it as well.

Fredrik Mörk
so I don't need the "this" keyword at all? do my get methods automatically act on the current instance of the class?
Alex
exactly, `this` is just more readable.
Anthony Forloney
ah, thank you! I appreciate the clarification!
Alex
Correct, leave off the "this." prefix and it will use methods and properties of the current instance.
Timothy Walters
It's especially good when it comes to code that has exposure to "beginners", as it makes purpose of a statement more explicit.
Romain
The only place you should use this is when you have a private field and a local variable with the same name. Unqualified the compiler will pick the local variable. So the following code needs to qualify one with "this": name = name;
Lasse V. Karlsen
What I meant is... the only place it is required to use "this", I agree with personal taste, so I didn't mean "do not use this in any other case".
Lasse V. Karlsen
that is not the only place this is required. if you want to return the current object you need to return this a good example of this would be overloading operator= to allow chaining
jk
Using 'this' here is redundant too, just look at the variable names :-)
Moron
A: 

I would say yes, it looks correct and easy to read. But isn't this something your TA should answer?

Sune Rievers
its extra credit, the TA and professor won't help on extra credit, not even talk to you about it. Which makes sense I guess.
Alex
Ok, fair enough :)
Sune Rievers
A: 

Usage of this keyword seems fine.

Though I believe for a class like Complex you should store the real and complex part as int properties and use them in the method, rather than using the methods GetRealPart() and GetComplexPart()

I would do it this way:

    class ComplexNumber
    {
        public int RealPart { get; set; }
        public int ComplexPart { get; set; }

        public ComplexNumber(int real, int complex)
        {
            this.RealPart = real;
            this.ComplexPart = complex;
        }

        public ComplexNumber Add(ComplexNumber c)
        {
            return new ComplexNumber(this.RealPart + c.RealPart, this.ComplexPart + c.ComplexPart);
        }
    }

The following is a scenario where this MUST be used, otherwise, the parameter and not the class member is considered for both LHS and RHS of the assignment.

public ComplexNumber(int RealPart, int ComplexPart)
        {
            RealPart = RealPart; // class member will not be assigned value of RealPart
            ComplexPart = ComplexPart;
        }
Rashmi Pandit
A: 
double realPartCaller = this.GetRealPart();

Even if you omit this from GetRealPart() it should still be okay. But the use of this makes it quite easy to read and understand when it comes to maintainer.

double realPartCaller = this.GetRealPart(); ==> bit more readable IMHO
double realPartCaller = GetRealPart();
aJ
+5  A: 

Use of the redundant this. is encouraged by the Microsoft coding standards as embodied in the StyleCop tool.

Steve Gilham
And it's discouraged by Resharper.
Carra
And it's discouraged by CodeRush. For which I hate it.
Vilx-
And it's discouraged by IDesign's coding standard: "65. Do not use the this reference unless invoking another constructor from within a constructor".That's how I use this, and I like R#'s default take on this.http://idesign.net/idesign/download/IDesign%20CSharp%20Coding%20Standard.zip
Martin R-L
+6  A: 

You can also to overload math operators, just like:

public static ComplexNumber operator +(ComplexNumber c1, ComplexNumber c2)
Rubens Farias
wow, that's nice, didn't know about that.
Alex
+1  A: 

I use this keyword only for variables and when there's an argument that has the same name as the private variable. i.e.

private String firstname;
public SetName(String firstname)
{
    this.firstname = firstname;
}
Raj
Conveniently this never happens because member fields always start with `_` while parameter names never start with `_`.
280Z28
A: 

I find myself more and more using the this keyword for both methods and properties on the current instance, as I feel it increases readability and maintainability. this is especially useful if your class also has static methods and/or properties, on which you of course can not use the this keyword, as these are not related to the current instance. By using this, you clearly see the difference.

To bring it even further, you should consider using the class name as a qualifier for static methods and properties, even within the class itself.

HKalnes
A: 

Just to add completeness to the answers - there is one case when the this keyword is mandatory. That's when you have a local variable (or a method parameter) that has the same name as a class member. In this case writing it without this will access the local variable and with this will set the class member. To illustrate:

class MyClass
{
    public int SomeVariable;

    public void SomeMethod()
    {
        int SomeVariable;

        SomeVariable = 4; // This assigns the local variable.
        this.SomeVariable = 6; // This assigns the class member.
    }
}

A couple things that follow from this:

  • Always avoid giving local variables the same name as class members (I admit, I don't always follow this myself);
  • Writing this in front of all member accesses acts like a safeguard. If you write a piece of code without it, and then later introduce a local variable with the same name and type as a class member, your code will still compile just fine, but will do something completely different (and probably wrong).

One instance though where I use the same names for method parameters as for class members is in constructors. I often write it like this:

class MyClass
{
    public int VariableA;
    public string VariableB;

    public MyClass(int VariableA, string VariableB)
    {
        this.VariableA = VariableA;
        this.VariableB = VariableB;
    }
}

In my opinion this makes the constructor clearer, because you immediately understand which parameter sets which class member.

Vilx-
A: 

If you follow the naming conventions, using this is rearlly neded:

class MyClass 
{ 
    public int _variableA; 
    public string _variableB; 

    public MyClass(int variableA, string variableB) 
    { 
        _variableA = variableA; 
        _variableB = variableB; 
    } 
}
Daniel
+3  A: 

Since you're now learning C# and asking about style, I'm going to show you several things that are wrong with the code you posted along with reasons.

Edit: I only responded to this because it looks like you actually working to figure this stuff out. Since that's the type of people I prefer to work with, I'm more critical simply because I hope it helps you get somewhere better as a result. :)

Structure name

  1. ComplexNumber is unnecessarily long. Note that none of Single, Double, Int32, Int64, etc. have Number in the name. This suggests Complex as a more appropriate name.
  2. Complex matches the naming already established in the .NET Framework.

Real and imaginary components

  1. GetRealPart() and GetComplexPart() should be get-only properties instead of methods.
  2. GetComplexPart() is misnamed because it is actually returning the imaginary part.
  3. Since the .NET framework already has a Complex structure, you shouldn't reinvent the naming. Therefore, unless you are in a position to redefine Framework conventions, the properties must be named Real and Imaginary.

Operations

If you look at existing examples like System.Windows.Vector, you see that math operations are implemented by providing a static method and an operator:

public static Point Add(Vector vector, Point point);
public static Point operator+(Vector vector, Point point);

Not surprisingly, this convention carried over to the System.Numerics.Complex structure:

public static Complex Add(Complex left, Complex right);
public static Complex operator +(Complex left, Complex right);

Summary

The result is clean, easy to verify, and behaves as everyone expects. The this keyword doesn't/can't appear because the methods are static.

public static Complex Add(Complex left, Complex right)
{
    return new Complex(left.Real + right.Real, left.Imaginary + right.Imaginary);
}

public static Complex operator +(Complex left, Complex right)
{
    return new Complex(left.Real + right.Real, left.Imaginary + right.Imaginary);
}
280Z28