CODEWITHSUNDEEP
c#optimization
Dante1986
Dante1986

Reputation: 59989

c# can i optimize this code?

I have this on a button:

private void Button_Click(object sender, RoutedEventArgs e)
{
    string s = "x12y04";

    //make new instance for MyMath class
    MyMath dRet01 = new MyMath();

    //use the doubleArrayXY (in MyMath class) to get doubble array back
    double[] retD = dRet01.doubleArrayXY(s);

    //use the calcResultFromDoubleArray (in MyMath class) to get result
    MyMath dRet02 = new MyMath();
    double result = dRet02.calcResultFromDoubleArray(retD[0], retD[1]);

    //DEBUG!
    /*
    string TEST1 = Convert.ToString(returnedDouble[0]);
    MessageBox.Show(TEST1);
    string TEST2 = Convert.ToString(returnedDouble[1]);
    MessageBox.Show(TEST2);
    string TEST3 = Convert.ToString(result);
    MessageBox.Show(TEST3);
   */


}

where the class "MyMath" is:

public double[] doubleArrayXY(string inputValue)
{
    //in case there are upper case letters, set all to lower
    string inpLow = inputValue.ToLower();

    //split the string on the Y (so this tech should also work for x89232y329)
    //so this will create res[0] that is x89232 and an res[1] that is 329
    string[] res = inpLow.Split(new string[] { "y" }, StringSplitOptions.None);

    //in the first string that looks like x89232, remove the x
    string resx = res[0].Replace("x", null);

    //now get the x value to a double
    double x = double.Parse(resx);
    //now get the y valye to a double
    double y = double.Parse(res[1]);

    //return in a double array the x and then the y (x=double[0] and y=double[1])
    return new double[] {x,y};
}

public double calcResultFromDoubleArray(double one, double two)
{
    return (one * two);
}

Now I know the part in the class that is "calcResultFromDoubleArray" is kind of useless at this point, but I want to make that do some extra stuff later on.

what I wonder about the most is in the main code where I make this new dRet10, and later on make dRet02.

I was thinking at first I could do something like this:

double result = dRet01.calcResultFromDoubleArray(retD[0], retD[1]);

So in that case I would not need to create a new instance of MyMath, but this does not work.

So I need to call a new instance for the class (like I did), or can I do this in a more elegant way? I'm still kind of new to C#, so I'm trying to learn how to program in a nice and elegant way, besides just making it work.

Upvotes: 0

Views: 155

Answers (5)

Chris
Chris

Reputation: 27627

It looks to me like the two methods on MyMath could (or possibly should) both be static since they rely on nothing at all outside the method. Quite often this is the case with things like Maths libraries. It seems others have said this too though.

In addition you may be better off to create a class or struct to represent your X/Y. It may be that it isn't appropriate but if it represents a thing then you might want a class to represent that thing as well. See for example the Point and PointF classes. I'd suggest one of these but they don't have the same precision that you are using (and your X/Y may not be points so it might not be appropriate)

Also the line you said didn't work:

double result = dRet01.calcResultFromDoubleArray(retD[0], retD[1]);

This should have worked with the code as shown. What error were you getting on it? dRet01 exists and so it should have worked just as well as creating a new instance. The comments that it should be static are most applicable but if you are new to C# I thought it worth pointing this out so you don't build up any wrong ideas about what is and isn't possible. :)

Upvotes: 0

Cameron S
Cameron S

Reputation: 2301

If you make your methods static you can do the following in your main class:

double result = MyMath.calcResultFromDoubleArray(MyMath.doubleArrayXY(s));

And change calcResultFromDoubleArray to take the array rather than two values (as its title suggests).

FYI you can also chain String operations because they return Strings as such:

string[] res = inputValue.ToLower().Split(new string[] { "y" }, StringSplitOptions.None);

No need to create double x and double y. Change the last part of the method to:

return new double[] {double.Parse(resx), double.Parse(res[1]};

While changes such as these (there are more, there often are) will be minimal increases in performance, they will increase it a bit (the most from the static part - new is relatively expensive).

Most importantly though, they make the code more readable and elegant.

Upvotes: 0

Danny
Danny

Reputation: 7518

In your code there is not really a point of creating an instance of MyMath class. You can make the methods static 

public static double[] doubleArrayXY(string inputValue) { ... }
public static double calcResultFromDoubleArray(double one, double two) { ... }

and call them like so

double[] retD = MyMath.doubleArrayXY(s);
double result = MyMath.calcResultFromDoubleArray(retD[0], retD[1]);

Upvotes: 0

davisoa
davisoa

Reputation: 5439

You could make your calcResultFromDoubleArray method static, and then call it via MyMath.calcResultFromDoubleArray(val1, val2)

Upvotes: 1

BrokenGlass
BrokenGlass

Reputation: 161012

Since your methods don't really use any other state information besides the parameters passed they probably should be static so you would not have to create any instances of your class at all:

double[] retD = MyMath.DoubleArrayXY(s);
double result = MyMath.CalcResultFromDoubleArray(retD[0], retD[1]);

If all of your methods in MyMath are static, declare the class itself static - just like the System.Math class, so you cannot create instances at all.

Upvotes: 5

Related Questions