A question of more efficient coding.

I've just completed another lab for my VB Programming class and I've noticed that it seems lengthy. I'm sure it's an overuse of if statements and just being new to the programming world. Being new, I only use what I've been taught in class as anything beyond that is beyond me. Please bare that in mind. :o

This program works (I believe the math is correct[?]), but ultimately, it looks somewhat sloppy to me. Any suggestions of methods for shortening code and making things less tedious for a newbie?

Code:

---

Public Class Form1
    'Jaycoba
    '11/09/2017

    'Array Data
    'Table1; AWG# (0-30) and Area
    Dim Table1() As Single =
      {105530, 83694, 66373, 52634, 41742, 33102,
        26250, 20816, 16509, 13094, 10381, 8234, 6529,
        5178.4, 4106.8, 3256.7, 2582.9, 2048.2, 1624.3,
        1288.1, 1021.5, 810.1, 642.4, 509.45, 404.01,
        320.4, 254.1, 201.5, 159.79, 126.72, 100.5}

    'Table 2; Wire Metals and Resistivity (0 - 5)
    Dim Table2() As Single =
        {9.9, 10.37, 14.7, 17, 47, 74}

    'Table 3; Fuze Sizes (in amperes, 0-49)
    Dim Table3() As Single =
        {1, 3, 6, 10, 15, 20, 25, 30, 35, 40, 45, 50,
        60, 70, 80, 90, 100, 110, 125, 150, 175, 200,
        225, 250, 300, 400, 450, 500, 600, 700, 800, 1000,
        1200, 1600, 2000, 2500, 3000, 4000, 5000, 6000}

    'Variables
    Dim R, A, l, p, I As Single
    Dim metal As Single
    Dim V As Single = 120

    'Retrieves Cross Sectional Area From Table1 by inputting a Wire Gage value from 0-30
    Function AreaOutput(ByVal AWG As Single) As Single
        Dim Area As Single
        'Outputs an error message if input # is over 30 and Exits Function on OKAY
        If AWG > 30 Then MsgBox("# Exceeds Highest Wire Gage", 0, "Error")
        If AWG > 30 Then Exit Function
        Area = Table1(AWG)
        AreaOutput = Area
    End Function

    'Retrieves Resistivity of Metal from Table 2 by selecting a radio button
    Function Resistivity(ByVal metal As Single) As Single
        Dim ResistanceOfMetal As Single
        ResistanceOfMetal = Table2(metal)
        Resistivity = ResistanceOfMetal
    End Function

    'Retrieves Fuse Size as a result of the calculation performed for the Current
    Function FuseSize(ByVal I As Integer) As Integer
        Dim count As Integer
        Dim Amperes As Single
        For count = 0 To 39
            If I < Table3(count) Then Amperes = Table3(count)
            FuseSize = Amperes
            If I > 6000 Then MsgBox("Fuze Size has reached its Max (6000). Try a different arrangement.", 0, "Error")
            If I > 6000 Then Exit For
            If FuseSize = Table3(count) Then Exit For
        Next
    End Function

    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click

        'Radio Buttons for the 6 types of metals
        If Ag.Checked = True Then metal = 0
        If Cu.Checked = True Then metal = 1
        If Au.Checked = True Then metal = 2
        If Al.Checked = True Then metal = 3
        If Ni.Checked = True Then metal = 4
        If Fe.Checked = True Then metal = 5

        'Assigning values
        p = Resistivity(metal)
        A = AreaOutput(MaskedTextBox1.Text)
        l = TextBox1.Text

        'Calculations for Resistance and Current, respectively.
        R = p * l / A
        I = V / R       


        'Output of the calculations
        TextBox2.Text = I
        TextBox3.Text = FuseSize(I)

    End Sub
End Class

---

The names of variables are kind of repeated (I.E. A, Area, AreaOutput, etc.). I wasn't sure what to name them to reduce confusion. I apologize if they're too similar.

I hope to use the knowledge gained from this site for other computer languages as well, so please don't spare any details!

Don't be afraid to put your if statements on multiple lines.

Code:

---

        If AWG > 30 Then MsgBox("# Exceeds Highest Wire Gage", 0, "Error")
        If AWG > 30 Then Exit Function

---

Should be

Code:

---

        If AWG > 30 Then
              MsgBox("# Exceeds Highest Wire Gage", 0, "Error")
              Exit Function
        End if

---

This ensures the condition is only checked once (rather than twice).

Same with this:

Code:

---

            If I > 6000 Then MsgBox("Fuze Size has reached its Max (6000). Try a different arrangement.", 0, "Error")
            If I > 6000 Then Exit For

---

The MessageBox and the Exit can both be inside the If block... again, only checking the condition once.


-tg

I'll add that your are using textboxes for output. Textboxes are things that the user can interact with. They would rightly expect that changes they make in there would have SOME impact, but that doesn't appear to be the case. If you just want to display information that the user shouldn't interact with, then use a label.

Another point that I'm not sure about is your use of the variable names Table1, Table2, and Table3. As a general rule, you should use variable names that are meaningful, and those....may not be, but they also might be. If those names correspond to the names of some table you used for reference, they might be meaningful to you, if not to us. However, if they don't correspond to anything, it would be better to use more descriptive names for variables, or else you'll end up forgetting what is what.

A third point I'll mention is that the use of the functions is a bit unusual. Class assignments tend to be a bit unusual, so that may explain it, but the functions don't really have a purpose, for the most part. For example, you have this method:

Code:

---

Function Resistivity(ByVal metal As Single) As Single
        Dim ResistanceOfMetal As Single
        ResistanceOfMetal = Table2(metal)
        Resistivity = ResistanceOfMetal
    End Function

---

which you could re-write into a single line:

Code:

---

Function Resistivity(ByVal metal As Single) As Single
        Return Table2(metal)
End Function

---

Which is a function that doesn't serve much of a purpose. You use it in a single place:

Code:

---

p = Resistivity(metal)

---

So you could just write:

Code:

---

p = Table2(metal)

---

and remove the method entirely.

Along those lines, assigning the return value to the method name is a pretty ancient way to return items from methods. If I remember right, it was the ONLY way that VB would return items up until .NET, but in .NET, there is the Return statement, which is more common. Neither one is more right, though, so you can do whichever you prefer, but you will see the Return statement FAR more often than you will see an assignment to the method name.

Quote:

---

Originally Posted by techgnome

Don't be afraid to put your if statements on multiple lines.

---

I will remember this and use it. This makes it look much nicer! Thank you!

Quote:

---

Originally Posted by Shaggy Hiker

I'll add that your are using textboxes for output... use a label.

---

It amuses me that people on a forum teach me so much more than a teacher at a college.

Quote:

---

...variable names Table1, Table2, and Table3...

---

This was done on purpose. It helped me reference the tables on the paper. I do need to work on naming everything appropriately though.

Quote:

---

...the functions don't really have a purpose... So you could just write:

Code:

---

p = Table2(metal)

---

and remove the method entirely.

---

As I was typing this code out, I thought the same thing. "I can just make p = table2(metal) and get the same result." The assignment calls for the use of functions to pull data from the tables (my fault for not specifying). I wasn't sure how else to do it, so it came out as seen in my first post.

To paraphrase the lab handout, it states, "Use a function to retrieve the Cross Sectional Area (using maskedtextbox2, Table 1). Use a function to retrieve the resistivity from the two letter code (using radiobuttons, Table 2). Use a function to determine the minimum size fuse (using calculations in code, Table 3.)

I don't know how to code this so that the functions are actually being purposeful. I think the teacher will be satisfied, he mainly checks that it works and that you've at least used what the lab calls for.

Quote:

---

... you will see the Return statement FAR more often than you will see an assignment to the method name.

---

This is just the way our teacher has taught us to code. I'll definitely be using the Return statement from now on, as long as our teacher doesn't have an issue with it.

Thank you for your help!

That's the thing about class work. Sometimes you are required to do things that don't make a whole lot of sense.

What you said about the table names is what I was getting at. They mean nothing to me, but they DO mean something to you, therefore they are good names. They speak to the person who understands the problem at hand. I have lots of variables that would seem particularly meaningless to a person who didn't understand the problem domain (such as ruid), but are pretty clear to a person who does know the domain.

I understand if you don't want to answer ALL of my questions Shaggy, but I have a lot more coming before the 2nd week of December when winter break starts!

I was going to make another thread, but I don't want to fill the forum with my nonsense. Sense this is another question of efficiency for a separate lab, I thought this thread was fitting anyways.

The lab is as follows;

Use a multi-value (jagged) array(s) to store the 1- or 2-character atomic symbol abbreviation, and also the atomic number for each element in the table. When an atomic symbol abbreviation is entered, print out all the atomic numbers and symbols for the elements in the same column of the periodic table as the selected element.

I wasn't sure if I should use information taught outside of class since I haven't talked to the teacher yet, so I haven't used what was shown to me in previous posts.

Code:

---

Public Class Form1
    'Jaycoba
    '11/09/2017

    'Arrays
    Dim elements(17)() As String
    Dim M As String

    Function print(ByVal letter As String) As String
        Dim abb As String
        Dim rows As Single
        For rows = 0 To 6
            abb &= elements(letter)(rows) & Chr(13) & Chr(10)
            print = abb
        Next
    End Function

    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click

        elements(0) = New String() {"H, 1.01", "Li, 6.94", "Na, 22.80", "K, 39.09", "Rb, 85.46", "Cs, 132.90", "Fr, (223)"}
        elements(1) = New String() {"Be, 9.01", "Mg, 24.30", "Ca, 40.07", "Sr, 87.62", "Ba, 137.32", "Ra, (226)"}
        elements(2) = New String() {"Sc, 44.86", "Y, 88.90", "La, 138.90", "Ac, (227)"}
        elements(3) = New String() {"Ti, 47.86", "Zr, 91.22", "Hf, 178.49", "Rf, (261)", "Ce, 140.11", "Th, 232.03"}
        elements(4) = New String() {"V, 50.94", "Nb, 92.90", "Ta, 180.94", "Db, (262)", "Pr, 140.90", "Pa, 231.03"}
        elements(5) = New String() {"Cr, 51.996", "Mo, 95.94", "W, 183.85", "Sg, (265)", "Nd, 144.24", "U, 238."}
        elements(6) = New String() {"Mn, 54.93", "Tc, 98.90", "Re, 168.20", "Bh, (264)", "Pm, 145", "Np, 237"}
        elements(7) = New String() {"Fe, 55.93", "Ru, 101.07", "Os, 190.23", "Hs, (269)", "Sm, 150.96", "Pu, 244"}
        elements(8) = New String() {"Co, 58.93", "Rh, 102.90", "Ir, 192.22", "Mt, (268)", "Eu, 151.96", "Am, 243"}
        elements(9) = New String() {"Ni, 58.69", "Pd, 106.42", "Pt, 195.08", "Gd, 157.25", "Cm, 247"}
        elements(10) = New String() {"Cu, 63.54", "Ag, 107.86", "Au, 196.96", "Tb, 158.92", "Bk, 247"}
        elements(11) = New String() {"Zn, 65.3", "Cd, 112.41", "Hg, 200.59", "Cn, (277)", "Dy, 162.50", "Cf, 251"}
        elements(12) = New String() {"B, 10.81", "Al, 25.98", "Ga, 69.73", "In, 114.81", "Tl, 204.38", "Ho, 164.93", "Es, 252"}
        elements(13) = New String() {"C, 12.01", "Si, 28.09", "Ge, 72.1", "Sn, 118.71", "Pb, 207.20", "Er, 167.26", "Fm, 257"}
        elements(14) = New String() {"N, 14.00", "P, 30.97", "As, 74.92", "Sb, 121.76", "Bi, 26.98", "Tm, 168.93", "Md, 258"}
        elements(15) = New String() {"O, 15.99", "S, 32.06", "Se, 78.96", "Te, 127.60", "Po, (209)", "Yb, 173.04", "No, 259"}
        elements(16) = New String() {"F, 18.99", "Cl, 35.45", "Br, 79.90", "I, 126.90", "At, (210)", "Lu, 14.96", "Lr, 262"}
        elements(17) = New String() {"He, 4.00", "Ne, 20.17", "Ar, 36.94", "Kr, 83.80", "Xe, 131.29", "Rn, (222)"}

        M = MaskedTextBox1.Text

        If M = "H" Or M = "Li" Or M = "Na" Or M = "K" Or M = "Rb" Or M = "Cs" Or M = "Fr" Then TextBox1.Text = print(0)

    End Sub
End Class

---

As you can see, I started typing out each M (shorter than writing out maskedtextbox1.text each time) equal to each individual atomic symbol. This was mostly to see if the function worked. Then I realized... that's a hell of a lot of if statements, but it works! Would it be more efficient to use two jagged arrays? What other methods can I use to retrieve the data?

If you are going use If statements to compare a variable/control value against multiple values, it is simpler to use a Select Case. One way would be like this:

Code:

---

Select Case MaskedTextBox1.Text
Case "H"
  MsgBox "MaskedTextBox1.Text equals 'H'"
Case "Li"
  MsgBox "MaskedTextBox1.Text equals 'Li'"
...
End Select

---

However, as it is this wont save much typing... thankfully you can use more than one value per case (simply separate them using commas), so you can do this:

Code:

---

Select Case MaskedTextBox1.Text
Case "H", "Li", "Na", "K", "Rb", "Cs", ...
  TextBox1.Text = print(0)
End Select

---

It would also be possible to loop the elements arrays, but as your data doesn't contain the letters on their own it would be fairly "messy" code, and so it may be better to avoid that.

Quote:

---

Originally Posted by si_the_geek

It would also be possible to loop the elements arrays, but as your data doesn't contain the letters on their own it would be fairly "messy" code, and so it may be better to avoid that.

---

So if I separated the atomic# from the symbol using 2 arrays, I can loop the arrays of symbols. Is it possible to refer the array containing symbols to the array containing the atomic numbers and have them print out accordingly? "H" with "1.01" and so on? I'm assuming they would have to be on separate lines, but I've been wrong many times before.

If you had 2 arrays you could output them like this:

Code:

---

            abb &= symbols(letter)(rows) & ", " & atomicNumbers(letter)(rows) & Chr(13) & Chr(10)

---

However, keeping the data in two arrays introduces makes it much harder to correctly read/write the code that sets them... and any issues there will almost certainly cause errors or (worse) bugs.

Very Insightful.

My last question for this particular lab. I tried to write a function that would take the input from maskedtextbox1.text and have it (attempt to) match the data in the jagged array using a for next loop. Let me see If I can arrange it close to how I had it + what you've explained.

Code:

---

    Dim elements(17)() As String
    Dim numbers(6)()
    Dim M As String
    Dim letter As String

    Function print(ByVal letter As String) As String
        Dim abb As String
        Dim column As Single
        Dim row2 As Single
        Dim row1 As Single
        For column = 0 To elements.GetUpperBound(0)
            For row1 = 0 To elements(column).GetUpperBound(0)
                If letter = elements(column)(row1) Then
                    For row2 = 0 To elements.GetUpperBound(0)
                        abb &= elements(column)(row2) & ", " & numbers(column)(row2)
                        print = abb
                    Next
                End If
            Next
        Next
    End Function

    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click

        elements(0) = New String() {"H", "Li", "Na", "K", "Rb", "Cs", "Fr"}
        numbers(0) = New String() {"1.01", "6.94", "22.80", "39.90", "85.46", "132.90", "(233)"}

        letter = MaskedTextBox1.Text

        TextBox1.Text = print(letter)

    End Sub
End Class

---

I've shortened it for easier viewing. I feel i butchered this one. For whatever reason, elements keeps coming back as null. What is it that I'm doing wrong? Would the idea of the loop checking for a string even work? I think I'll just stick the "Select Case" for now. I do hope to get this other function to work the way I'm imagining it though.

I would say that starting a new thread for a new question is the better way to go, even if you do feel like it is cluttering things. The forum then acts somewhat as a resource for others.

I'll keep the threads coming then! :thumb:

I'll keep tinkering with this one and see if I can't come up with a solution. As for now, onto the next lab.