CODEWITHSUNDEEP
javascriptjquery
A.Gecu
A.Gecu

Reputation: 93

How to does Code abbreviation in this program

I have just started learning javascript

Here is my code below.
I think it's too long so I need a help with making it shorter.

<html xmlns="http://www.w3.org/1999/xhtml">
  <head>
    <title></title>
  </head>
  <body>
    <button onclick="myFunction()">do it</button>
    <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
    <script>
      var x = document.createElement("INPUT");
      x.setAttribute("type", "text");
      x.setAttribute("value", "");
      x.setAttribute("id", "top2");
      document.body.appendChild(x);
      var x = document.createElement("INPUT");
      x.setAttribute("type", "text");
      x.setAttribute("value", "");
      x.setAttribute("id", "top");
      document.body.appendChild(x);
      var x = document.createElement("SELECT");
      x.setAttribute("id", "mySelect");
      document.body.appendChild(x);
      $("#mySelect").append($("<option value='toplama'></option>").text("Toplama"));
      $("#mySelect").append($("<option value='çıkarma'></option>").text("Çıkarma"));

      function myFunction() {
        var sayi1 = document.getElementById('top').value;
        var sayi2 = document.getElementById('top2').value;
        var sonuc;
        if ($("#mySelect").val() == "toplama") {
          sonuc = parseInt(sayi1) + parseInt(sayi2);
          alert(sonuc);
        } else if ($("#mySelect").val() == "çıkarma") {
          sonuc = parseInt(sayi1) - parseInt(sayi2);
          alert(sonuc);
        }
      }
    </script>
  </body>
</html>

How can I improve this javascript code?
What should I do?

Upvotes: 0

Views: 60

Answers (2)

n4rzul
n4rzul

Reputation: 4078

Just scanning the code quickly I notice a couple of things that can be improved which I will list below. However, just because a particular piece of code is long, does not make it bad code. Breaking up pieces of code can make it more readable even if its a little longer. Readability is more important than brevity.

1 - Be consistent
 You are using raw javascript by using document.getElementById and then later you use the jquery selectors. document.getElementById('top').value vs $("#top"). If you are going to be using jquery, use it everywhere. Jquery takes good care of cross platform nasties as well for you

2 - Whitespace and readability
 Insert some whitespace between sections of your code. Separate the pieces of code that construct top, top2 and mySelect. Where to insert whitespace is a bit of an art form, there are no hard and fast rules, but use some intuition and your code wont read like one long difficult to read sentence.

3 - Cache variables
 You create top, top2 and myselect but don't store them in variables and you are looking them up again in myFunction. Although this is a trivial example, get into the habit of caching your variables to avoid costly jquery locator lookups. Do this even for code that is trivial, since you never know when it is actually going to make a difference until it is too late.

4 - Avoid duplication
 Duplication is the bane of any programmers life. Viciously and ferociously get rid of it at every opportunity you get. The alert(sonuc); is in both the if and the else but can be moved to after it. No need to duplicate it in both.

I have rewritten your code a little just to illustrate the points I have made.

In closing, improving code is an ongoing, never ending process that is sometimes a little subjective. The best way to improve the way you write code is to keep practising. A very good piece of advise that was given to me is to read other peoples code that are better than you. There are tons of respositories on github with well respected projects that you can peruse. The added benefit is you will most likely learn a lot more than just how to improve your code.

Best of luck

<html xmlns="http://www.w3.org/1999/xhtml">
    <head>
        <title></title>
    </head>
    <body>
        <button onclick="myFunction()">do it</button>
        <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
        <script>
        var body =  $("body");

        var input1 = $("<input>")
          .attr("type",  "text")          
          .attr("id", "top2");

        body.append(input1);

        var input2 = $("<input>")
         .attr("type",  "text")          
         .attr("id", "top");

        body.append(input2);

        var operation = $("<select>")
         .attr("type",  "text")          
         .attr("id", "mySelect");

        body.append(operation);

        operation.append($("<option>").attr("value", "toplama").text("toplama"));
        operation.append($("<option>").attr("value", "Çıkarma").text("Çıkarma"));

        function myFunction() {
          var sayi1 = $("#top").value();
          var sayi2 = $("#top").value();
          var sonuc;
          if ($("#mySelect").val() == "toplama") {
            sonuc = parseInt(sayi1) + parseInt(sayi2);                    
          }
          else if ($("#mySelect").val() == "çıkarma") {
            sonuc = parseInt(sayi1) - parseInt(sayi2);                    
          }
          alert(sonuc);
        }
        </script>
    </body>
    </html>

Upvotes: 1

Kiran Shinde
Kiran Shinde

Reputation: 5982

There are two things I want to point.

  1. Cache jQuery elements those are used multiple times. (To increase performance)

  2. Why to create elements form javascript. Create them directly form html

Your code should look like

<body>
    <button onclick="myFunction()">do it</button>
    <input type="text" value="" id="top2" />
    <input type="text" value="" id="top" />
    <select id="myselect">
        <option value='toplama'>Toplama</option>
         <option value='çıkarma'>Çıkarma</option>
    </select>
    <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
    <script>
        function myFunction() {
            var sayi1 = document.getElementById('top').value,
                sayi2 = document.getElementById('top2').value,
                sonuc,
                $dropDown = $("#mySelect");

            if ($dropDown.val() == "toplama") {
                sonuc = parseInt(sayi1) + parseInt(sayi2);
                alert(sonuc);
            }
            else if ($dropDown.val() == "çıkarma") {
                sonuc = parseInt(sayi1) - parseInt(sayi2);
                alert(sonuc);
            }
        }
    </script>
</body>

Upvotes: 0

Related Questions