Jump to content

Programing Practice

Windows7ge
8 hours ago, Nuluvius said:

If you're ever in doubt then review the documentation: NumericUpDown.Value.

Your link taught me something new. There's different kinds of decimal types. They're all still defined as decimals but they're all different. I'm to assume this is also applicable to integers, strings, etc. I wonder what makes any one a better choice or the only choice over the others.

Link to comment
Share on other sites

Link to post
Share on other sites

@Nuluvius

I've hit a wall. No matter how much I google I cannot locate the next step which is probably an indicator that I've done something wrong (or all of it from the beginning). I incorporated var which seamlessly did what you explained. I just popped it in and put my cursor over top of it and it behaved just like when I had the decimal there. So that cool. I used the const to control the default value of the input boxes like you explained. That also went in without a hitch. However. The function. I knew this was going to go badly. We went over functions in VB but all of it was explained in something like 1 or two classes so I didn't have enough time to understand them. I don't know if what I wrote below is a function or a poorly constructed method. I think you should be able to tell where my idea was heading.

 

My first idea was to assign the * & / to a variable so I wouldn't have to use an If statement to determine which to use. However a few hours of googling gave me no results telling me if it was even possible to represent +, -, *, /, with variables in an equation and have that equation function. My own experimenting led to nothing as well.

 

My next plan was to use an If statement. However the If didn't want to work with a string as in If(action = *) or If(action = /) then in the following lines I could simply go value1 / value2 or value1 * value2

 

My third idea was to use a bool which you can bee below. This worked...sort of but now I have two issues.

 

1. I cannot alter the bool to True/False from outside the public decimal

2. I can put data into the public decimal but I cannot get data back out

 

I tried experimenting with public void, public static decimal nothing made any difference so I've come to the conclusion that I'm extremely far off from what I should have done. I tried to research C# functions but nothing I found gave examples that I understood. Most examples used code that I'm not experienced with so I didn't know what I was looking at.

 

You may also notice some bits of code have gone missing or comments are incorrect. That's because the whole project is "under construction" and I haven't cleaned anything up because I'm stuck.

public decimal powerResult(decimal value1, decimal value2, decimal answer)
        { 
            bool division = true;

            if (division == true)
            {
                answer = value1 / value2;
                return answer;
            }
            else answer = value1 * value2;
                return answer;
        }

        private void btnCalculate1_Click(object sender, EventArgs e)
        { 
            var value1 = Convert.ToDecimal(nudWatts1.Value); // watts = nudWatts1.Value
            var value2 = Convert.ToDecimal(nudAmps1.Value); // amps = nudAmps1.Value
        }

        private void btnClear1_Click(object sender, EventArgs e)
        {
            const decimal valueDefault = 0.001m;

            nudWatts1.Value = valueDefault;
            nudAmps1.Value = valueDefault;
            lblOutputVolts.Text = string.Empty;
        }

 

Link to comment
Share on other sites

Link to post
Share on other sites

1 hour ago, Windows7ge said:

My first idea was to assign the * & / to a variable so I wouldn't have to use an If statement to determine which to use. However a few hours of googling gave me no results telling me if it was even possible to represent +, -, *, /, with variables in an equation and have that equation function. My own experimenting led to nothing as well.

You can't store operators in variables, the closest thing that you can do is use lambdas or delegates.

1 hour ago, Windows7ge said:

My next plan was to use an If statement. However the If didn't want to work with a string as in If(action = *) or If(action = /) then in the following lines I could simply go value1 / value2 or value1 * value2

 

My third idea was to use a bool which you can bee below. This worked...sort of but now I have two issues.

 

1. I cannot alter the bool to True/False from outside the public decimal

2. I can put data into the public decimal but I cannot get data back out

I don't really understand the mathematics that are going on in your code but if you need something to happen based on a Boolean condition then why not simply pass that in as a parameter? Perhaps it might be beneficial to explain the mathematical premise to me or point me in the direction of where I can read up on it.

const decimal valueDefault = 0.001m;

A slight note on standards, constants usually start with a capitol.

The single biggest problem in communication is the illusion that it has taken place.

Link to comment
Share on other sites

Link to post
Share on other sites

1 minute ago, Nuluvius said:

You can't store operators in variables, the closest thing that you can do is use lambdas or delegates.

In my quest to find answers I came across a website mentioning lambdas. I didn't look into it but I recognize the word.

 

7 minutes ago, Nuluvius said:

Perhaps it might be beneficial to explain the mathematical premise to me

If I'm going to use a DRY structure the function that processes my information needs to be universal enough to accept the different equations necessary to the application.

 

The function needs to be capable of Multiplication and Division depending on which inputs are being used. Such as:

 

Watts divided by Amps equals Volt

Volts times Amps equals Watts

Watts divided by Volts equals Amps

 

If a variable cannot represent a operator then I have to write separate math problems. One for Multiplication and one for Division. Then I need a way to tell the program weather to multiply the two numbers or divide the two numbers.

 

An If statement with the following code determining which operation takes place seems the simplest. I just need to figure out:

1. How to control the bool which will determine weather to multiply or divide the numbers.

2. How to get the answer back out. I can put information into the function but I can't seem to get an answer back out.

19 minutes ago, Nuluvius said:

if you need something to happen based on a Boolean condition then why not simply pass that in as a parameter?

As far as my understanding goes I think I've already done that. The bool in in the if statement and it is controlling weather we multiply or divide which i believe is a parameter I'm just not sure how to alter its value from outside the void. Below is a more explanatory example:

public decimal powerResult(decimal value1, decimal value2, decimal answer)
        { 
            bool division = true; // This is the bool, if division is true then below the two values will be divided.

            if (division == true)
            {
                answer = value1 / value2;
                return answer;
            }
            else answer = value1 * value2; // False and the two numbers will be multiplied
                return answer;
        }

        private void btnCalculate1_Click(object sender, EventArgs e)
        { 
          // In order to determine if we multiply or divide I need to be able to control the bool (True/False) from here.
            var value1 = Convert.ToDecimal(nudWatts1.Value); // watts = nudWatts1.Value
            var value2 = Convert.ToDecimal(nudAmps1.Value); // amps = nudAmps1.Value
        }

 

Link to comment
Share on other sites

Link to post
Share on other sites

Could you not look at method overloading in this case? Then the correct method will be chosen implicitly right?

// Gigabyte 990FXA-UD3 // AMD FX-8320 CPU @ 4.3 Ghz (7-21.5 Multiplier) 200.90mhz FSB CPU-Z Validated // Kraken X40 AIO - 2x140mm Push-Pull // 4GB Corsair Vengeance LP - 8GB Avexir Core Series Red 1760Mhz // Sapphire R9 Fury Nitro 1130mhz/4GB 1025mhz (Effective) GPU-Z Validation // Corsair SP2500 2.1 & Microlab Solo 9C Speakers // Corsair K90 Silver - Cherry MX Red & Blue LEDs // EVGA SuperNova 850w G2

Link to comment
Share on other sites

Link to post
Share on other sites

3 hours ago, Mkfish said:

Could you not look at method overloading in this case? Then the correct method will be chosen implicitly right?

I don't know what this is or how it's done but it's worth looking into.

 

My one other idea is to try and use a module file. From what I was taught from a class in college it should allow me to do what I'm trying to here. Weather or not it'd be well written is another story.

Link to comment
Share on other sites

Link to post
Share on other sites

11 hours ago, Windows7ge said:

The function needs to be capable of Multiplication and Division depending on which inputs are being used. Such as:

 

Watts divided by Amps equals Volt

Volts times Amps equals Watts

Watts divided by Volts equals Amps

 

If a variable cannot represent a operator then I have to write separate math problems. One for Multiplication and one for Division. Then I need a way to tell the program weather to multiply the two numbers or divide the two numbers.

 

An If statement with the following code determining which operation takes place seems the simplest. I just need to figure out:

1. How to control the bool which will determine weather to multiply or divide the numbers.

2. How to get the answer back out. I can put information into the function but I can't seem to get an answer back out.

So something such as this for example:

namespace ConsoleApp1
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine(PowerResult(10, 5, true));
            Console.WriteLine(PowerResult(10, 5, false));
            Console.ReadKey();
        }

        private static decimal PowerResult(decimal value1, decimal value2, bool division)
        {
            return division ? value1 / value2 : value1 * value2;
        }
    }
}

Which could be cleaned up into:

namespace ConsoleApp1
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine(PowerResult(10, 5));
            Console.WriteLine(PowerResult(10, 5, false));
            Console.ReadKey();
        }

        private static decimal PowerResult(decimal value1, decimal value2, bool division = true) => division ? value1 / value2 : value1 * value2;
    }
}

You may want to read about the Ternary Operator and Expression-bodied members.

2 hours ago, Windows7ge said:

My one other idea is to try and use a module file. From what I was taught from a class in college it should allow me to do what I'm trying to here. Weather or not it'd be well written is another story.

There's no reason that something like this can't go into it's own class. My only argument against that at this point may be that the whole application is far too simple to justify it.

5 hours ago, Mkfish said:

Could you not look at method overloading in this case? Then the correct method will be chosen implicitly right?

Indeed you could, if it makes things more clear. OP may like to read about Overloading and Overriding as well as more general Polymorphism within the context of C#.

The single biggest problem in communication is the illusion that it has taken place.

Link to comment
Share on other sites

Link to post
Share on other sites

@Nuluvius

That's a lot more cleanly written than what I was doing.

Looking at it I assume the state of the bool will determine weather the values are multiplied or divided.

 

If I make that public I should be able to assign values to the variables without issue.

I'll still have the dilemma of how to control the bool and how to get an answer back out. I don't plan to perform the final operations inside the function. I would ideally like to call the function, input the values I want it to use, determine the operator with the bool, have it run then return the result and allow me to output it to the form from the private void where the request originated. (I hope I'm using these terminologies correctly)

 

And I'll start reading the links you posted.

Link to comment
Share on other sites

Link to post
Share on other sites

1 hour ago, Windows7ge said:

Looking at it I assume the state of the bool will determine weather the values are multiplied or divided.

Correct.

1 hour ago, Windows7ge said:

If I make that public I should be able to assign values to the variables without issue.

Why do you think that it needs to be public?

1 hour ago, Windows7ge said:

I'll still have the dilemma of how to control the bool and how to get an answer back out. I don't plan to perform the final operations inside the function.

I really don't see the problem. Pass whatever variables to it that you need to as the arguments and assign the return value to whatever you need; my example shows you how to do this.

 

As side point, the arguments:

decimal value1, decimal value2

Should each have a meaningful name that fits into your narrative.

The single biggest problem in communication is the illusion that it has taken place.

Link to comment
Share on other sites

Link to post
Share on other sites

47 minutes ago, Nuluvius said:

Why do you think that it needs to be public?

...do I not need it to be public if I want to input data from another void?

 

In VB a Private Sub had to be made public in order for anything inside of it to be seen from alternates subs. If I wanted to perform an operation inside a Sub that was separate from another or multiple (kind of like this instance) sub(s) I would have to make the Sub public in order to pass data into it.

 

...or I'm just spewing more mis-teachings that shouldn't be used and have more proper alternatives.

 

51 minutes ago, Nuluvius said:

I really don't see the problem. Pass whatever variables to it that you need to as the arguments and assign the return value to whatever you need; my example shows you how to do this.

OH!, ok, I think I understand what I'm looking at now. Your example numbers represent the data I want to pass into the function. They are separated by commas and each correspond to the (decimal, decimal, bool) which is inside the function.

 

55 minutes ago, Nuluvius said:

Should each have a meaningful name that fits into your narrative.

When it comes to creativity like making up names, I'm bad. Just look at my LTT forum username. I made it after an operating system just because I couldn't think of anything better. I know the variable names should hold some meaning to represent the data they will hold. It makes sense to do so and makes the code easier to read but in this instance the names have to be universal. Since each variable in the function has to represent Volts, Amps, & Watts all at the same time. If I come up with something I will replace them.

 

Despite the frustrations I'm enjoying myself quite a bit. Each time I overcome something that I don't understand I get a feeling of acomplishment.

Link to comment
Share on other sites

Link to post
Share on other sites

2 hours ago, Windows7ge said:

...do I not need it to be public if I want to input data from another void?

 

In VB a Private Sub had to be made public in order for anything inside of it to be seen from alternates subs. If I wanted to perform an operation inside a Sub that was separate from another or multiple (kind of like this instance) sub(s) I would have to make the Sub public in order to pass data into it.

 

...or I'm just spewing more mis-teachings that shouldn't be used and have more proper alternatives.

Consider the following:

Module Module1

    Sub Main()
        Console.WriteLine(SomeString)
        Console.ReadKey()
    End Sub


    Private Function SomeString() As String
        Return "Some String"
    End Function

End Module

I'd suggest that perhaps you should go back and review the links that I previously gave you on Accessibility Levels and Access Modifiers.

2 hours ago, Windows7ge said:

OH!, ok, I think I understand what I'm looking at now. Your example numbers represent the data I want to pass into the function. They are separated by commas and each correspond to the (decimal, decimal, bool) which is inside the function.

Yes you've got it xD

2 hours ago, Windows7ge said:

When it comes to creativity like making up names, I'm bad. Just look at my LTT forum username. I made it after an operating system just because I couldn't think of anything better. I know the variable names should hold some meaning to represent the data they will hold. It makes sense to do so and makes the code easier to read but in this instance the names have to be universal. Since each variable in the function has to represent Volts, Amps, & Watts all at the same time. If I come up with something I will replace them.

I understand and usually when one is faced with this sort of conundrum it can be indicative of a possible opportunity to change the design in order to improve upon the narrative.

2 hours ago, Windows7ge said:

Despite the frustrations I'm enjoying myself quite a bit. Each time I overcome something that I don't understand I get a feeling of acomplishment.

You've captured the essence of the journey quite well I feel xD

 

It's possible that I may not be able to respond for a while as I have an operation tomorrow morning :/

The single biggest problem in communication is the illusion that it has taken place.

Link to comment
Share on other sites

Link to post
Share on other sites

@Nuluvius

With a little troubleshooting because the code wasn't exactly plug 'n' play (I kept getting the error that the program could not implicitly convert type decimal to type string and with some alterations it even reversed itself and said could not convert string to decimal so that was a conundrum.) it now works. In theory I can just assign the form object values to variables and send them to the function for the other two private voids. Cleaner, less repetitive, and although the program is too small to see any real performance gain from the improved efficiency it is technically less work for the system.

private static decimal PowerResult(decimal powerInput1, decimal powerInput2, bool division = true) => division ? powerInput1 / powerInput2 : powerInput1 * powerInput2;

private void btnCalculate1_Click(object sender, EventArgs e)
{ 
var watts = Convert.ToDecimal(nudWatts1.Value); // watts = nudWatts1.Value
var amps = Convert.ToDecimal(nudAmps1.Value); // amps = nudAmps1.Value

lblOutputVolts.Text = Convert.ToString(PowerResult(watts, amps, true));
}

 

Link to comment
Share on other sites

Link to post
Share on other sites

13 hours ago, Windows7ge said:

With a little troubleshooting because the code wasn't exactly plug 'n' play (I kept getting the error that the program could not implicitly convert type decimal to type string and with some alterations it even reversed itself and said could not convert string to decimal so that was a conundrum.) it now works. In theory I can just assign the form object values to variables and send them to the function for the other two private voids. Cleaner, less repetitive, and although the program is too small to see any real performance gain from the improved efficiency it is technically less work for the system.

That's great, I'm happy that you've managed to get it working :D

 

A cleanup you could make would be to change:

lblOutputVolts.Text = Convert.ToString(PowerResult(watts, amps, true));

To:

lblOutputVolts.Text = PowerResult(watts, amps, true).ToString();

Getting rid of the external conversion call and using the Decimal.ToString() method instead.

The single biggest problem in communication is the illusion that it has taken place.

Link to comment
Share on other sites

Link to post
Share on other sites

@Nuluvius

I noticed something interesting. When I call the function since the bool is by default set to true it appears I don't have to define its value when I want to divide. Even if I multiply and the bool changes to false (which means it will multiply the numbers) if I then immediately choose to divide numbers again by calling the function the bool always defaults to true even if I don't tell it to return to true.

 

I assume this is because it says (bool = true) as a parameter in the function but I didn't know its value is reset each time the function is called.

 

Should I still define its value when sending data into the function? It doesn't appear to affect the application either way.

Link to comment
Share on other sites

Link to post
Share on other sites

15 minutes ago, Windows7ge said:

I noticed something interesting. When I call the function since the bool is by default set to true it appears I don't have to define its value when I want to divide. Even if I multiply and the bool changes to false (which means it will multiply the numbers) if I then immediately choose to divide numbers again by calling the function the bool always defaults to true even if I don't tell it to return to true.

 

I assume this is because it says (bool = true) as a parameter in the function but I didn't know its value is reset each time the function is called.

That's right, each time you call that function everything will be 'reset' in effect. Or rather more accurately it will be as if you never even called it (as no state is actually transferred or known about within it's scope between calls).

 

The Boolean parameter is somewhat special because it's what's known as a Default Parameter or Optional Argument.

20 minutes ago, Windows7ge said:

Should I still define its value when sending data into the function? It doesn't appear to affect the application either way.

It's debatable really. I don't really care for default parameters that much because I think that they add unnecessary complexity and convolute the design. I prefer to be explicit... in other words I'd rather overload if it served to add better clarity.

 

In this context however you might find it better to make a few functions; one that is very general (such as the one you already have) and others that are very specific that call the more general one. You can then name them in such a way as to capture their behaviour and intention. Your specific functions can encapsulate whatever arrangement of arguments (such as the specific Boolean state) of the more general function that actually does the real work. One could argue that the calculation function, in its 'generalness' is itself an implementation detail and therefore a good candidate for such encapsulation.

 

I don't know and can't really imagine how this would look or fit into your design so I'd say that if you want to do it then do and just see how you feel about the way that it looks afterwards.

The single biggest problem in communication is the illusion that it has taken place.

Link to comment
Share on other sites

Link to post
Share on other sites

4 hours ago, Nuluvius said:

I don't know and can't really imagine how this would look or fit into your design so I'd say that if you want to do it then do and just see how you feel about the way that it looks afterwards.

If I'm understanding you correctly then in my opinion to have a general function that determines the state of something which then calls more functions to perform a more specific operation seems excessive for this application. I did notice your example function which I have implemented since it uses a bool as part of the function I believe it is limited to two separate operations (multiply the numbers or divide them in this instance). To separate the bool from the function I expect would allow for higher complexity such as if I have more than two separate operations that I want to have occur for specific inputs.

 

I think the application is too simple to benefit from making multiple functions. In an attempt to remove the bool and make the code "clearer" I think would have the adverse effect. If I was trying to use 3 or more math equations then it would probably be beneficial to separate the bool from the rest of the operation and use it externally to determine which function gets used but as it is now I think it's about as clear as it's going to get. I'm still new to C# and it only took me a couple hours to understand exactly how your example code works. Clarity I don't think can get any simpler than that unless Cyclomatic complexity is the concern but even then it's one line of code and it's not going in very many directions.

 

To top everything off I don't think it would be smart of me to take and use every single piece of information you give me. If I did then my code would start appearing the same way you would have written it yourself and that's just kind of scumbag-ish. To copy someone else's style. You could arguably call it plagiarism. So long as I'm not falling into any bad practices and I'm satisfied with the clarity of my code then I'd call it good enough.

 

Speaking of clarity, one little thing is bothering me. We touched on it once before:

const decimal ValueDefault = 0.001m;

I have this written 3 times. Once in each void that requires this default value.

I have the idea in my head that I could attempt to put it inside its own void, or function which in theory would allow me to write it just once then I could assign it to the three voids than need it.

 

It just kind of annoys me that it's so repetitive. Seems unnecessary.

Link to comment
Share on other sites

Link to post
Share on other sites

1 hour ago, Windows7ge said:

If I'm understanding you correctly then in my opinion to have a general function that determines the state of something which then calls more functions to perform a more specific operation seems excessive for this application. I did notice your example function which I have implemented since it uses a bool as part of the function I believe it is limited to two separate operations (multiply the numbers or divide them in this instance). To separate the bool from the function I expect would allow for higher complexity such as if I have more than two separate operations that I want to have occur for specific inputs.

Not quite, you've got it the wrong way round. An example may help to better convey the concept:

private static decimal DivideSomeNumbers(decimal value1, decimal value2)
{
  // We encapsulate the specific setup/configuration of the general function
  return SomeGeneralCalculation(value1, value2);
}

private static decimal MultipleSomeNumbers(decimal value1, decimal value2)
{
  // We encapsulate the specific setup/configuration of the general function
  return SomeGeneralCalculation(value1, value2, false);
}

private static decimal SomeGeneralCalculation(decimal value1, decimal value2, bool division = true) => division ? value1 / value2 : value1 * value2;
1 hour ago, Windows7ge said:

To top everything off I don't think it would be smart of me to take and use every single piece of information you give me. If I did then my code would start appearing the same way you would have written it yourself and that's just kind of scumbag-ish. To copy someone else's style. You could arguably call it plagiarism. So long as I'm not falling into any bad practices and I'm satisfied with the clarity of my code then I'd call it good enough.

Much of what I've shown you has not been very much to do with my own personal subjective style. These concepts and practices are for the most part objective and quite standard within the industry (where things are done right that is).

1 hour ago, Windows7ge said:

I have this written 3 times. Once in each void that requires this default value.

I have the idea in my head that I could attempt to put it inside its own void, or function which in theory would allow me to write it just once then I could assign it to the three voids than need it.

 

It just kind of annoys me that it's so repetitive. Seems unnecessary.

You can simply consolidate those into one single constant at the class scope.

The single biggest problem in communication is the illusion that it has taken place.

Link to comment
Share on other sites

Link to post
Share on other sites

Just now, edward30 said:

He could store the operator as a char and use a switch statement in a function(a, b, op). But uh... he probably shouldn't. xD

Nor should anyone really O.o

 

The correct method for doing this sort of thing would likely be the Strategy Pattern since we are interested in changing the behaviour of a thing.

The single biggest problem in communication is the illusion that it has taken place.

Link to comment
Share on other sites

Link to post
Share on other sites

2 hours ago, Nuluvius said:

Not quite, you've got it the wrong way round. An example may help to better convey the concept:

Ah, that's what you meant. That is another way of going about it.

2 hours ago, Nuluvius said:

You can simply consolidate those into one single constant at the class scope.

Something along the lines of:

public partial class ConvertVoltsWattsAmps : Form
    {
        public ConvertVoltsWattsAmps()
        {
            InitializeComponent();
        }
    
    const decimal ValueDefault = 0.001m;
  
    . . .
    }

or do I need to put it inside of a void?

Link to comment
Share on other sites

Link to post
Share on other sites

On 10/08/2017 at 10:01 PM, Windows7ge said:

or do I need to put it inside of a void?

No how you have it is mostly fine. Though it's best practice to place all class scope variables and constants at the top and to be explicit about their visibility.

The single biggest problem in communication is the illusion that it has taken place.

Link to comment
Share on other sites

Link to post
Share on other sites

3 hours ago, Nuluvius said:

No how you have it is mostly fine. Though it's best practice to place all class scope variables and constants at the top and to be explicit about their visibility.

Well with that in mind the program is complete. I can only think of one thing I leaned in VB class that didn't go against everything you taught me over the last two and a half weeks.

Link to comment
Share on other sites

Link to post
Share on other sites

9 hours ago, Windows7ge said:

Well with that in mind the program is complete.

Yay xD

9 hours ago, Windows7ge said:

I can only think of one thing I leaned in VB class that didn't go against everything you taught me over the last two and a half weeks.

I'm intrigued, what is it?

The single biggest problem in communication is the illusion that it has taken place.

Link to comment
Share on other sites

Link to post
Share on other sites

9 hours ago, Nuluvius said:

I'm intrigued, what is it?

someInputBox.Text = String.Empty(), quite literally the only thing I learned in college that you agreed with. Money well spent *sarcasm*.

 

Here's the final code fully assembled:

namespace ConvertVoltsWattsAmps
{
    public partial class ConvertVoltsWattsAmps : Form
    {
        public ConvertVoltsWattsAmps()
        {
            InitializeComponent();
        }

        const decimal ValueDefault = 0.001m; // Constant to reset the value of the input fields

        // Function: When called, takes two inputs and based on the bool (true/false) will either divide or multiply the numbers
        private static decimal PowerResult(decimal powerInput1, decimal powerInput2, bool division = true) => division ? powerInput1 / powerInput2 : powerInput1 * powerInput2;

        private void btnCalculate1_Click(object sender, EventArgs e)
        { 
            var watts = Convert.ToDecimal(nudWatts1.Value); // Converts the value contained in nudWatts1 to Decimal and assigns it to the variable watts
            var amps = Convert.ToDecimal(nudAmps1.Value); // Converts the value contained in nudAmps1 to Decimal and assigns it to the variable amps

            lblOutputVolts.Text = PowerResult(watts, amps, true).ToString(); // Calls the function, inputs parameters, returns answer, converts to string, outputs to form
        }

        private void btnClear1_Click(object sender, EventArgs e)
        {
            nudWatts1.Value = ValueDefault;
            nudAmps1.Value = ValueDefault;
            lblOutputVolts.Text = string.Empty;
        }

        private void btnCalculate2_Click(object sender, EventArgs e)
        {
            var volts = Convert.ToDecimal(nudVolts2.Value);
            var amps = Convert.ToDecimal(nudAmps2.Value);

            lblOutputWatts.Text = PowerResult(volts, amps, false).ToString();
        }

        private void btnClear2_Click(object sender, EventArgs e)
        {
            nudVolts2.Value = ValueDefault;
            nudAmps2.Value = ValueDefault;
            lblOutputWatts.Text = string.Empty;
        }

        private void btnCalculate3_Click(object sender, EventArgs e)
        {
            var watts = Convert.ToDecimal(nudWatts3.Value);
            var volts = Convert.ToDecimal(nudVolts3.Value);

            lblOutputAmps.Text = PowerResult(watts, volts, true).ToString();
        }

        private void btnClear3_Click(object sender, EventArgs e)
        {
            nudVolts3.Value = ValueDefault;
            nudWatts3.Value = ValueDefault;
            lblOutputAmps.Text = string.Empty;
        }
    }
}

48 lines of code (not including comments, blank lines, or the pages of code that make up the form itself) this is many times less code than when I made the program in VB but I also completely rebuilt the form itself (My first attempt included using 4 separate forms. The mainForm, then one for each separate measure of power) I'm using a tab menu this time around which easily accommodated all of my objects in one small mainForm window.

Link to comment
Share on other sites

Link to post
Share on other sites

14 hours ago, Windows7ge said:

someInputBox.Text = String.Empty(), quite literally the only thing I learned in college that you agreed with. Money well spent *sarcasm*.

xD

public partial class ConvertVoltsWattsAmps : Form
{
  private const decimal ValueDefault = 0.001m; // Constant to reset the value of the input fields
  
  public ConvertVoltsWattsAmps()
  {
    InitializeComponent();
  }
  
  ...

Class scope constants and variables come first in a class and their visibility is declared explicitly. I'd argue that the comment, in this case, is unnecessary as it doesn't add any value and moreover it's somewhat misleading; 'Constant to reset the value of the input fields':

  • We already know that it's a constant, it's obvious and we shouldn't be using comments to state the obvious.
  • It does not reset the values itself, it represents a specific value that has been chosen to be used as a default in this particular case.
  • What input fields? That is immediately ambiguous.
var watts = Convert.ToDecimal(nudWatts1.Value); // Converts the value contained in nudWatts1 to Decimal and assigns it to the variable watts
var amps = Convert.ToDecimal(nudAmps1.Value); // Converts the value contained in nudAmps1 to Decimal and assigns it to the variable amps

lblOutputVolts.Text = PowerResult(watts, amps, true).ToString(); // Calls the function, inputs parameters, returns answer, converts to string, outputs to form

Again, we are using comments to state the very obvious here; the narrative is clearly offered by the code itself.

private void btnClear1_Click(object sender, EventArgs e)
{
  nudWatts1.Value = ValueDefault;
  nudAmps1.Value = ValueDefault;
  lblOutputVolts.Text = string.Empty;
}

...

private void btnClear2_Click(object sender, EventArgs e)
{
  nudVolts2.Value = ValueDefault;
  nudAmps2.Value = ValueDefault;
  lblOutputWatts.Text = string.Empty;
}

...

private void btnClear3_Click(object sender, EventArgs e)
{
  nudVolts3.Value = ValueDefault;
  nudWatts3.Value = ValueDefault;
  lblOutputAmps.Text = string.Empty;
}

When we start repeating ourselves in such a way it's often a sign that we need to refactor our code. We can improve this part of the code by extracting the common behaviour out and putting it into another method which we may then pass our objects to:

private static void Reset(NumericUpDown powerInput1, NumericUpDown powerInput2, Control output)
{
  powerInput1.Value = ValueDefault;
  powerInput2.Value = ValueDefault;
  output.Text = string.Empty;
}

We have captured the behaviour and encapsulated it within a meaningfully named method that describes what's going on which can then be used as follows:

private void btnClear1_Click(object sender, EventArgs e)
{
  Reset(nudWatts1, nudAmps1, lblOutputVolts);
}

...

private void btnClear2_Click(object sender, EventArgs e)
{
  Reset(nudVolts2, nudAmps2, lblOutputWatts);
}

...

private void btnClear3_Click(object sender, EventArgs e)
{
  Reset(nudVolts3, nudWatts3, lblOutputAmps);
}

Thus removing the repeated code and greatly improving the clarity of the narrative.

15 hours ago, Windows7ge said:

I also completely rebuilt the form itself (My first attempt included using 4 separate forms. The mainForm, then one for each separate measure of power) I'm using a tab menu this time around which easily accommodated all of my objects in one small mainForm window.

Indeed this must be where the numerical suffix on some of your controls originates from. This kind of thing is also indicative of a issue with the design.

 

It certainly seems to me as through there's further opportunity for improvement here. It may be possible to have a single Form and a single User Control; the User Control could consist of your two inputs and your output and could be instantiated taking whatever names you want to set them up to be. Your Form could then be responsible for instantiating that User Control a number of times and also passing over a calculation strategy. You can host that User Control however you like within your Form i.e. in a TabControl if you feel that to be the most appropriate (I've not seen your GUI to be best placed to speculate).

 

This may also be a good first step in moving towards a more formal architecture such as Model View Controller (MVC) or Model VIew VIewModel (MVVM).

The single biggest problem in communication is the illusion that it has taken place.

Link to comment
Share on other sites

Link to post
Share on other sites

5 hours ago, Nuluvius said:

Class scope constants and variables come first in a class and their visibility is declared explicitly.

Example code helps A LOT when you're trying to explain something to me that I still don't completely understand. Here's something interesting I just discovered. Using a constant with a protection level of private but at the class scope still allows all private voids within that class to see it. I don't think VB is even capable of this.

 

5 hours ago, Nuluvius said:

I'd argue that the comment, in this case, is unnecessary as it doesn't add any value and moreover it's somewhat misleading; 'Constant to reset the value of the input fields':

I'm describing the reason for it's existence rather than what the line itself means. Since I don't have any serious plans for the distribution of my programs I kind of have the habit of writing comments in a way that explains to myself what it does because my memory is rather fickle and if I don't write a program at least 2 or 3 weeks apart from each other I can't remember how to do it the next time I want to. Then I can reference past projects and the comments refresh my memory.

 

5 hours ago, Nuluvius said:

Indeed this must be where the numerical suffix on some of your controls originates from. This kind of thing is also indicative of a issue with the design.

When I began construction I tried to think of word based names to represent the objects but the names I thought of would have become hard to distinguish from each other and redundant. I know using numbers isn't something I should use but again the mental block I mentioned a while ago, creating names isn't one of my strong suits. It's easy enough when I don't have duplicate buttons that do the exact same thing and in this is where there's more room for improvement.

 

5 hours ago, Nuluvius said:

I've not seen your GUI to be best placed to speculate

I do see how it might be possible to use private voids within the tab menu and based on which tab the user is on it could change what the button controls do. This could reduce the amount of code, reduce repetitive buttons, and make the naming scheme easier.

Here's the program. The other two tabs are nearly identical except for names, what the controls represent, and what controls the buttons affect:

Screenshot_1.png.d45aa0d778cc5902758c4b4e1fa62eb4.png

Link to comment
Share on other sites

Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now


×