Consejos necesarios para refactorizar / limpiar mi código JavaScript -- javascript camp codereview Relacionados El problema

Advice needed in refactoring/cleaning up my javascript code


4
vote

problema

Español

Soy bastante nuevo para JavaScript. He escrito un sitio web que funciona según lo diseñado. Sin embargo, tengo una fuerte sensación de que mi código podría ser fuertemente refactorado. ¿Alguien puede ver algo, además del espaciado de línea, que podría hacer para limpiar mi código?

  public class MainActivity extends Activity {      /**      * Called when the activity is first created.      */      @Override     public void onCreate(Bundle savedInstanceState)     {         super.onCreate(savedInstanceState);         setContentView(R.layout.activity_main);         setBoard();     }      int check[][];     int i,j;     Button b[][];     int player=0;     TextView textView;     Button newGame;      // Set up the game board.     private void setBoard()     {          b = new Button[4][4];         check = new int[4][4];           textView = (TextView) findViewById(R.id.textview1);          newGame = (Button) findViewById(R.id.newgame);         newGame.setOnClickListener (new View.OnClickListener(){              public void onClick(View v)             {                 if(newGame.isEnabled())                 {                     textView.setText("Click button to start!");                     player=0;                     setBoard();                 }             }         });          b[1][3] = (Button) findViewById(R.id.one);         b[1][2] = (Button) findViewById(R.id.two);         b[1][1] = (Button) findViewById(R.id.three);           b[2][3] = (Button) findViewById(R.id.four);         b[2][2] = (Button) findViewById(R.id.five);         b[2][1] = (Button) findViewById(R.id.six);           b[3][3] = (Button) findViewById(R.id.seven);         b[3][2] = (Button) findViewById(R.id.eight);         b[3][1] = (Button) findViewById(R.id.nine);          for (i = 1; i <= 3; i++) {             for (j = 1; j <= 3; j++)                 check[i][j] = 2;         }          // add the click listeners for each button          for (i = 1; i <= 3; i++) {             for (j = 1; j <= 3; j++) {                 b[i][j].setOnClickListener(new MyClickListener(i, j));                 if (!b[i][j].isEnabled()) {                     b[i][j].setText("");                     b[i][j].setEnabled(true);                 }             }         }     }      class MyClickListener implements View.OnClickListener     {         int x;         int y;           public MyClickListener(int x, int y)         {             this.x = x;             this.y = y;         }            public void onClick(View view)         {             if (b[x][y].isEnabled())             {                 b[x][y].setEnabled(false);                 if (player == 0)                 {                     b[x][y].setText("X");                     check[x][y] = 0;                     player = 1;                     checkBoard();                 } else                 {                     b[x][y].setText("O");                     check[x][y] = 1;                     player = 0;                     checkBoard();                 }             }         }     // check the board to see if someone has won     private boolean checkBoard() {             boolean gameOver = false;             if (( check[1][1] == 0 && check[2][2] == 0 &&  check[3][3] == 0)                     || ( check[1][3] == 0 && check[2][2] == 0 &&  check[3][1] == 0)                     || ( check[1][2] == 0 &&  check[2][2] == 0 &&  check[3][2] == 0)                     || ( check[1][3] == 0 &&  check[2][3] == 0 &&  check[3][3] == 0)                     || ( check[1][1] == 0 &&  check[1][2] == 0 &&  check[1][3] == 0)                     || ( check[2][1] == 0 &&  check[2][2] == 0 &&  check[2][3] == 0)                     || ( check[3][1] == 0 &&  check[3][2] == 0 &&  check[3][3] == 0)                     || ( check[1][1] == 0 &&  check[2][1] == 0 &&  check[3][1] == 0)) {                 textView.setText("Player 1: You win!");                 gameOver = true;             } else if (( check[1][1] == 1 && check[2][2] == 1 && check[3][3] == 1)                     || ( check[1][3] == 1 && check[2][2] == 1 && check[3][1] == 1)                     || ( check[1][2] == 1 && check[2][2] == 1 && check[3][2] == 1)                     || ( check[1][3] == 1 && check[2][3] == 1 && check[3][3] == 1)                     || ( check[1][1] == 1 && check[1][2] == 1 && check[1][3] == 1)                     || ( check[2][1] == 1 && check[2][2] == 1 && check[2][3] == 1)                     || ( check[3][1] == 1 && check[3][2] == 1 && check[3][3] == 1)                     || ( check[1][1] == 1 &&  check[2][1] == 1 && check[3][1] == 1)) {                 textView.setText("Player 2: You Win!");                 gameOver = true;             } else {                 boolean empty = false;                 for (i = 1; i <= 3; i++) {                     for (j = 1; j <= 3; j++) {                         if (check[i][j] == 2) {                             empty = true;                             break;                         }                     }                 }                 if (!empty) {                     gameOver = true;                     textView.setText("Game over. It's a draw!");                  }             }if(gameOver)                  for(i=1;i<=3;i++)                 {                     for(j=1;j<=3;j++)                     {                         b[i][j].setEnabled(false);                     }                 }              return gameOver;         }     }  } 0  

Todo mi código se puede ver en http://codepen.io/angstd/pen/ropca < / a>.

Original en ingles

I am fairly new to Javascript. I have written a website that works as designed. However, I have a strong feeling that my code could be heavily refactored. Can anyone see anything, besides line spacing, that I could do to clean up my code?

var i1 = document.createElement("img"); var i2 = document.createElement("img"); var i3 = document.createElement("img"); var i4 = document.createElement("img"); var i5 = document.createElement("img"); var i6 = document.createElement("img"); var i7 = document.createElement("img"); var i8 = document.createElement("img"); var i9 = document.createElement("img"); i1.type = "image"; i2.type = "image"; i3.type = "image"; i4.type = "image"; i5.type = "image"; i6.type = "image"; i7.type = "image"; i8.type = "image"; i9.type = "image"; var floatingButton1 = document.getElementById('floatingButton1'); var floatingButton2 = document.getElementById('floatingButton2'); var floatingButton3 = document.getElementById('floatingButton3'); var floatingButton4 = document.getElementById('floatingButton4'); var floatingButton5 = document.getElementById('floatingButton5'); var floatingButton6 = document.getElementById('floatingButton6'); var floatingButton7 = document.getElementById('floatingButton7'); var floatingButton8 = document.getElementById('floatingButton8'); var floatingButton9 = document.getElementById('floatingButton9'); floatingButton1.style.paddingLeft="35px"; floatingButton2.style.paddingLeft="35px"; floatingButton3.style.paddingLeft="35px"; floatingButton4.style.paddingLeft="35px"; floatingButton5.style.paddingLeft="35px"; floatingButton6.style.paddingLeft="35px"; floatingButton7.style.paddingLeft="35px"; floatingButton8.style.paddingLeft="35px"; floatingButton9.style.paddingLeft="35px"; floatingButton1.style.paddingTop="5px"; floatingButton2.style.paddingTop="5px"; floatingButton3.style.paddingTop="5px"; floatingButton4.style.paddingTop="5px"; floatingButton5.style.paddingTop="5px"; floatingButton6.style.paddingTop="5px"; floatingButton7.style.paddingTop="5px"; floatingButton8.style.paddingTop="5px"; floatingButton9.style.paddingTop="5px";  function clearButtons() {     i = 1;     while (i < 10) {         var floatingButtonToClear = document.getElementById('floatingButton' + i);         floatingButtonToClear.style.backgroundImage="url('Images/whitesquare.png')";              i = i + 1;     } }  function makeOptionButtons(a, b) {     i = a;     while (a <= b) {         var floatingButtonOptionsClear = document.getElementById('floatingButton' + a);         floatingButtonOptionsClear .style.backgroundImage="url('Images/buttonUnclicked.png')";          a = a + 1;     } }  function clearThumbnails() {          if (floatingButton1.hasChildNodes())     {     floatingButton1.removeChild(i1);     }     if (floatingButton2.hasChildNodes())     {     floatingButton2.removeChild(i2);     }     if (floatingButton3.hasChildNodes())     {     floatingButton3.removeChild(i3);     }     if (floatingButton3.hasChildNodes())     {     floatingButton3.removeChild(i3);     }     if (floatingButton4.hasChildNodes())     {     floatingButton4.removeChild(i4);     }     if (floatingButton5.hasChildNodes())     {     floatingButton5.removeChild(i5);     }     if (floatingButton6.hasChildNodes())     {     floatingButton6.removeChild(i6);     }     if (floatingButton7.hasChildNodes())     {     floatingButton7.removeChild(i7);     }     if (floatingButton8.hasChildNodes())     {     floatingButton8.removeChild(i8);     }     if (floatingButton9.hasChildNodes())     {     floatingButton9.removeChild(i9);     } }    function showImageOnClick1(a) {     clearThumbnails()     clearButtons()     document.getElementById(a).style.visibility = 'visible'; }  function showImageOnClick2(a, b) {     clearThumbnails()     clearButtons()     document.getElementById(a).style.visibility = 'visible';     document.getElementById(b).style.visibility = 'hidden'; }  function showImageOnClick3(a, b, c) {     clearThumbnails()     clearButtons()     document.getElementById(a).style.visibility = 'visible';     document.getElementById(b).style.visibility = 'hidden';     document.getElementById(c).style.visibility = 'hidden'; }  function showImageOnClick4(a, b, c, d) {     clearThumbnails()     clearButtons()     document.getElementById(a).style.visibility = 'visible';     document.getElementById(b).style.visibility = 'hidden';     document.getElementById(c).style.visibility = 'hidden';     document.getElementById(d).style.visibility = 'hidden'; }  function Door() {     clearThumbnails()     clearButtons()     makeOptionButtons(1, 4);         i1.src = "Images/tnBB1.png";     floatingButton1.appendChild(i1);      i2.src = "Images/tnBB1.png";     floatingButton2.appendChild(i2);      i3.src = "Images/tnBB1.png";     floatingButton3.appendChild(i3);      i4.src = "Images/tnBB1.png";     floatingButton4.appendChild(i4);     i1.onclick = function () {         showImageOnClick4('Door1', 'Door2', 'Door3', 'Door4')     };     i2.onclick = function () {         showImageOnClick4('Door2', 'Door1', 'Door3', 'Door4')     };     i3.onclick = function () {         showImageOnClick4('Door3', 'Door1', 'Door2', 'Door4')     };     i4.onclick = function () {         showImageOnClick4('Door4', 'Door1', 'Door2', 'Door3')     }; }  function Trim() {     clearThumbnails()     clearButtons()     makeOptionButtons(2, 4);     i2.src = "Images/tnS1.png";     i2.innerHTML = "1";     floatingButton2.appendChild(i2);      i3.src = "Images/tnS2.png";     floatingButton3.appendChild(i3);      i4.src = "Images/tnR3 - Copy.png";     floatingButton4.appendChild(i4);     i2.onclick = function () {         showImageOnClick3('Trim1', 'Trim2', 'Trim3')     };     i3.onclick = function () {         showImageOnClick3('Trim2', 'Trim1', 'Trim3')     };     i4.onclick = function () {         showImageOnClick3('Trim3', 'Trim1', 'Trim2')     }; }  function Roof() {     clearThumbnails()     clearButtons()     makeOptionButtons(3, 5);        i3.src = "Images/tnT3.png";     floatingButton3.appendChild(i3);      i4.src = "Images/tnBB3.png";     floatingButton4.appendChild(i4);      i5.src = "Images/tnBB1.png";     floatingButton5.appendChild(i5);      i3.onclick = function () {         showImageOnClick3('Roof1', 'Roof2', 'Roof3')     };     i4.onclick = function () {         showImageOnClick3('Roof2', 'Roof1', 'Roof3')     };     i5.onclick = function () {         showImageOnClick3('Roof3', 'Roof1', 'Roof2')     }; }  function Siding() {     clearThumbnails()     clearButtons()     makeOptionButtons(4, 6);      i4.src = "Images/tnBB2.png";     floatingButton4.appendChild(i4);      i5.src = "Images/tnBB311.png";     floatingButton5.appendChild(i5);      i6.src = "Images/tnBB112.png";     floatingButton6.appendChild(i6);     i4.onclick = function () {         showImageOnClick3('Siding1', 'Siding2', 'Siding3')     };     i5.onclick = function () {         showImageOnClick3('Siding2', 'Siding1', 'Siding3')     };     i6.onclick = function () {         showImageOnClick3('Siding3', 'Siding1', 'Siding2')     }; }  function Stone() {     clearThumbnails()     clearButtons()     makeOptionButtons(5, 7);      i5.src = "Images/tnBB1.png";     floatingButton5.appendChild(i5);      i6.src = "Images/tnR3 - Copy14.png";     floatingButton6.appendChild(i6);      i7.src = "Images/tnBB1.png";     floatingButton7.appendChild(i7);     i5.onclick = function () {         showImageOnClick3('Stone1', 'Stone2', 'Stone3')     };     i6.onclick = function () {         showImageOnClick3('Stone2', 'Stone1', 'Stone3')     };     i7.onclick = function () {         showImageOnClick3('Stone3', 'Stone1', 'Stone2')     }; }  function BB() {     clearThumbnails()     clearButtons()     makeOptionButtons(6, 9);      i6.src = "Images/tnBB1.png";     floatingButton6.appendChild(i6);      i7.src = "Images/tnBB117.png";     floatingButton7.appendChild(i7);      i8.src = "Images/tnBB218.png";     floatingButton8.appendChild(i8);      i9.src = "Images/tnBB319.png";     i9.type = "image";     floatingButton9.appendChild(i9);     i6.onclick = function () {         showImageOnClick4('BB1', 'BB2', 'BB3', 'BB4')     };     i7.onclick = function () {         showImageOnClick4('BB2', 'BB1', 'BB3', 'BB4')     };     i8.onclick = function () {         showImageOnClick4('BB3', 'BB1', 'BB2', 'BB4')     };     i9.onclick = function () {         showImageOnClick4('BB4', 'BB1', 'BB2', 'BB3')     }; } 

All of my code can be seen at http://codepen.io/angstd/pen/ropca.

  
 
 

Lista de respuestas

7
 
vote
vote
La mejor respuesta
 

No estoy tan experimentado en JavaScript, así que ten cuidado con los errores de sintaxis, pero mirándolo de lo que puedo leer ...

este ...

  var i1 = document.createElement("img"); var i2 = document.createElement("img"); var i3 = document.createElement("img"); var i4 = document.createElement("img"); var i5 = document.createElement("img"); var i6 = document.createElement("img"); var i7 = document.createElement("img"); var i8 = document.createElement("img"); var i9 = document.createElement("img"); i1.type = "image"; i2.type = "image"; i3.type = "image"; i4.type = "image"; i5.type = "image"; i6.type = "image"; i7.type = "image"; i8.type = "image"; i9.type = "image"; var floatingButton1 = document.getElementById('floatingButton1'); var floatingButton2 = document.getElementById('floatingButton2'); var floatingButton3 = document.getElementById('floatingButton3'); var floatingButton4 = document.getElementById('floatingButton4'); var floatingButton5 = document.getElementById('floatingButton5'); var floatingButton6 = document.getElementById('floatingButton6'); var floatingButton7 = document.getElementById('floatingButton7'); var floatingButton8 = document.getElementById('floatingButton8'); var floatingButton9 = document.getElementById('floatingButton9'); floatingButton1.style.paddingLeft="35px"; floatingButton2.style.paddingLeft="35px"; floatingButton3.style.paddingLeft="35px"; floatingButton4.style.paddingLeft="35px"; floatingButton5.style.paddingLeft="35px"; floatingButton6.style.paddingLeft="35px"; floatingButton7.style.paddingLeft="35px"; floatingButton8.style.paddingLeft="35px"; floatingButton9.style.paddingLeft="35px"; floatingButton1.style.paddingTop="5px"; floatingButton2.style.paddingTop="5px"; floatingButton3.style.paddingTop="5px"; floatingButton4.style.paddingTop="5px"; floatingButton5.style.paddingTop="5px"; floatingButton6.style.paddingTop="5px"; floatingButton7.style.paddingTop="5px"; floatingButton8.style.paddingTop="5px"; floatingButton9.style.paddingTop="5px";   

podría ser esto:

  var imageArray = []; var floatingButtonArray = []; for (i = 0; i < 9; i++) {      imageArray[i] = document.createElement("img");     imageArray[i].type = "image";     floatingButtonArray[i] = document.getElementById('floatingButton'+(i+1));     floatingButtonArray[i].style.paddingLeft = "35px";     floatingButtonArray[i].style.paddingTop = "5px"; }   

Cada vez que haces cosas como esta ...

  var silverlightCheck = Silverlight.isInstalled("1.0"); if (silverlightcheck) // do stuff 0  

Podrías estar haciendo esto en su lugar:

  var silverlightCheck = Silverlight.isInstalled("1.0"); if (silverlightcheck) // do stuff 1  

Solo esto reduciría tanto tu código, no tienes idea. Para los ejemplos dados, irías de 90 líneas a 17. Sin embargo, no es el número de líneas lo que es importante. Lo importante es la cantidad de repetición en su código.

"Pero espera, tengo otras imágenes y llaman I101, I102 y I103!"

Ponga los nombres de las imágenes en una matriz. Divida las matrices de imágenes dependiendo del significado. Si esos i101 son especiales, póngalos en una matriz separada. Pero en general, haz lo mejor para evitar la repetición.

 

I'm not so experienced in javascript, so beware syntax errors, but looking at it from what I can read...

This...

var i1 = document.createElement("img"); var i2 = document.createElement("img"); var i3 = document.createElement("img"); var i4 = document.createElement("img"); var i5 = document.createElement("img"); var i6 = document.createElement("img"); var i7 = document.createElement("img"); var i8 = document.createElement("img"); var i9 = document.createElement("img"); i1.type = "image"; i2.type = "image"; i3.type = "image"; i4.type = "image"; i5.type = "image"; i6.type = "image"; i7.type = "image"; i8.type = "image"; i9.type = "image"; var floatingButton1 = document.getElementById('floatingButton1'); var floatingButton2 = document.getElementById('floatingButton2'); var floatingButton3 = document.getElementById('floatingButton3'); var floatingButton4 = document.getElementById('floatingButton4'); var floatingButton5 = document.getElementById('floatingButton5'); var floatingButton6 = document.getElementById('floatingButton6'); var floatingButton7 = document.getElementById('floatingButton7'); var floatingButton8 = document.getElementById('floatingButton8'); var floatingButton9 = document.getElementById('floatingButton9'); floatingButton1.style.paddingLeft="35px"; floatingButton2.style.paddingLeft="35px"; floatingButton3.style.paddingLeft="35px"; floatingButton4.style.paddingLeft="35px"; floatingButton5.style.paddingLeft="35px"; floatingButton6.style.paddingLeft="35px"; floatingButton7.style.paddingLeft="35px"; floatingButton8.style.paddingLeft="35px"; floatingButton9.style.paddingLeft="35px"; floatingButton1.style.paddingTop="5px"; floatingButton2.style.paddingTop="5px"; floatingButton3.style.paddingTop="5px"; floatingButton4.style.paddingTop="5px"; floatingButton5.style.paddingTop="5px"; floatingButton6.style.paddingTop="5px"; floatingButton7.style.paddingTop="5px"; floatingButton8.style.paddingTop="5px"; floatingButton9.style.paddingTop="5px"; 

could be this:

var imageArray = []; var floatingButtonArray = []; for (i = 0; i < 9; i++) {      imageArray[i] = document.createElement("img");     imageArray[i].type = "image";     floatingButtonArray[i] = document.getElementById('floatingButton'+(i+1));     floatingButtonArray[i].style.paddingLeft = "35px";     floatingButtonArray[i].style.paddingTop = "5px"; } 

Any time you do stuff like this...

function clearThumbnails() {          if (floatingButton1.hasChildNodes())     {     floatingButton1.removeChild(i1);     }     if (floatingButton2.hasChildNodes())     {     floatingButton2.removeChild(i2);     }     if (floatingButton3.hasChildNodes())     {     floatingButton3.removeChild(i3);     }     if (floatingButton3.hasChildNodes())     {     floatingButton3.removeChild(i3);     }     if (floatingButton4.hasChildNodes())     {     floatingButton4.removeChild(i4);     }     if (floatingButton5.hasChildNodes())     {     floatingButton5.removeChild(i5);     }     if (floatingButton6.hasChildNodes())     {     floatingButton6.removeChild(i6);     }     if (floatingButton7.hasChildNodes())     {     floatingButton7.removeChild(i7);     }     if (floatingButton8.hasChildNodes())     {     floatingButton8.removeChild(i8);     }     if (floatingButton9.hasChildNodes())     {     floatingButton9.removeChild(i9);     } } 

You could be doing this instead:

function clearThumbnails(){     for(i = 0; i < 9; i++){         if(floatingButtonArray[i].hasChildNodes()){             floatingButtonArray.removeChild(imageArray[i]);         }     } } 

This alone would shrink your code so much, you have no idea. For the examples given, you'd go from 90 lines to 17. However, it's not the number of lines that's important. What's important is the amount of repetition in your code.

"But wait, I have other images and they're called i101, i102 and i103!"

Put the image names in an array. Split the image arrays depending on meaning. If those i101 and so are special, put them in a seperate array. But in general, do your best to avoid repetition.

 
 
8
 
vote

Para agregar a lo que dijo @pimgd, en lugar de usar un bucle 99887766555443312 y 99887776655443314 y usted debe usar un var silverlightCheck = Silverlight.isInstalled("1.0"); if (silverlightcheck) // do stuff 5 Loop.

Cada vez que vea este patrón:

  var silverlightCheck = Silverlight.isInstalled("1.0"); if (silverlightcheck) // do stuff 6  

Reemplazarlo con esto:

  var silverlightCheck = Silverlight.isInstalled("1.0"); if (silverlightcheck) // do stuff 7  

Como aparte, en JavaScript a menos que intente explícitamente crear una variable global (que casi siempre no debe), debe usar la palabra clave 998877766655443318 . Por ejemplo, su var silverlightCheck = Silverlight.isInstalled("1.0"); if (silverlightcheck) // do stuff 9 debe ser $('.playbtn').show(); checkEncodingFunc();0 .

 

To add to what @Pimgd said, instead of using a while loop in clearButtons and makeOptionButtons and you should use a for loop.

Whenever you see this pattern:

var i = 1; while (i < 10) {     // ...     i = i + 1; } 

replace it with this:

for (var i = 1; i < 10; i++) {     // ... } 

As an aside, in Javascript unless you explicitly intend to create a global variable (which you nearly always should not), then you should use the var keyword. For example your i = 1; should be var i = 1;.

 
 
 
 
3
 
vote

agregando a lo que otros ya han dicho.


en lugar de estas funciones:

  $('.playbtn').show(); checkEncodingFunc();1  

Podría simplificar usar solo uno:

  $('.playbtn').show(); checkEncodingFunc();2  

Podría reemplazar todas sus llamadas a $('.playbtn').show(); checkEncodingFunc();3 . $('.playbtn').show(); checkEncodingFunc();4 Con este, funcionará para todos ellos, aunque solo se nombra el primer argumento ( $('.playbtn').show(); checkEncodingFunc();5 ).


También podría simplificar este tipo de código:

  $('.playbtn').show(); checkEncodingFunc();6  

Puede crear una función de ayuda que creará una función:

  $('.playbtn').show(); checkEncodingFunc();7  

que simplificará la creación de las funciones ONCLICK con una sola línea y menos código de la placa de la caldera:

  $('.playbtn').show(); checkEncodingFunc();8  
 

Adding to what others have already said.


Instead of these functions:

function showImageOnClick1(a) {     clearThumbnails()     clearButtons()     document.getElementById(a).style.visibility = 'visible'; } function showImageOnClick2(a, b) {     // ... } function showImageOnClick3(a, b, c) {     // ... } function showImageOnClick4(a, b, c, d) {     clearThumbnails()     clearButtons()     document.getElementById(a).style.visibility = 'visible';     document.getElementById(b).style.visibility = 'hidden';     document.getElementById(c).style.visibility = 'hidden';     document.getElementById(d).style.visibility = 'hidden'; } 

You could simplify to use only one:

function showImageOnClick(a) {     clearThumbnails()     clearButtons()     document.getElementById(a).style.visibility = 'visible';     for (var i = 1; i < arguments.length; ++i) {         var arg = arguments[i];         document.getElementById(arg).style.visibility = 'hidden';     } } 

You could replace all your calls to showImageOnClick1 .. showImageOnClick4 with this one, it will work for all of them even though only the first argument is named (a).


You could also simplify this kind of code:

i1.onclick = function () {     showImageOnClick4('Door1', 'Door2', 'Door3', 'Door4') }; i2.onclick = function () {     showImageOnClick4('Door2', 'Door1', 'Door3', 'Door4') }; i3.onclick = function () {     showImageOnClick4('Door3', 'Door1', 'Door2', 'Door4') }; i4.onclick = function () {     showImageOnClick4('Door4', 'Door1', 'Door2', 'Door3') }; 

You can create a helper function that will create a function:

function showImageOnClickMaker() {     var args = arguments;     return function() { showImageOnClick.apply(this, args); } } 

Which will simplify creating the onclick functions with a single line and less boilerplate code:

i1.onclick = showImageOnClickMaker('Door1', 'Door2', 'Door3', 'Door4'); i2.onclick = showImageOnClickMaker('Door2', 'Door1', 'Door3', 'Door4'); i3.onclick = showImageOnClickMaker('Door3', 'Door1', 'Door2', 'Door4'); i4.onclick = showImageOnClickMaker('Door4', 'Door1', 'Door2', 'Door3'); 
 
 
1
 
vote

No voy a refactar mucho código, pero solo ofreceré algún consejo general.

Lo primero que un principiante es necesario acostumbrarse a separar sus datos de sus funciones. La forma más sencilla de a esto es romper sus datos en un obecto global (en este caso). Piense en las partes de su aplicación y descomponen así ..

(lo voy a alar aquí, pero obtendrás la idea)

  var appData={   floatingButtons:{     count:9     ,style:{       paddingLeft:'35px'       ,paddingTop:'5px'     }     ,clearStyle:{       backgroundImage:"url(blah de blah)"     }     ,unclickedStyle:{       backgroundImage:"url(blah de blah)"     }     ,elements:{     }   }   /// blah, blah, more like this };   // now lets make some buttons function makeFloatingButtons(){   var buttons=appData.floatingButtons;   function getButton(x){     var rv=buttons.elements['floatingButton'+x]=document.getElementById('floatingButton'+x);     return rv;   }   function styleButton(el){     var styles=buttons.style;     var o=el.style;     for (var i in styles) {//see how this is magic? just declare new styles when you need them       o[i]=styles[i];     }   }   for (var x=0;x<buttons.count;x+=1) {     var btn=getButton(x);     styleButton(btn);   } }  function clearFloatingButtons(){   var buttons=appData.floatingButtons;   //easy   for (var i in buttons.elements) {     var style=buttons.elements[i].style;     for (var j in buttons.clearStyle) {       style[j]=buttons.clearStyle[j];// more magic     }   } }   

Concentrado en la estructura de sus datos y la generación de sus funciones, eventualmente aprenderá cómo mover sus funciones a ese objeto global y luego comenzar a pensar en una manera de OOP (una vez que vea cómo las funciones y los datos pueden vivir como hermano y hermana si estás organizado).

 

I am not going to refactor much code, but just offer some general advice.

The first thing as a beginner is you need to get used to separating your data from your functions. The simplest way to to this is to break down your data into a global obect (in this case). Think of the parts of your app and break it down like so..

(I'm going to wing it here but you will get the idea)

var appData={   floatingButtons:{     count:9     ,style:{       paddingLeft:'35px'       ,paddingTop:'5px'     }     ,clearStyle:{       backgroundImage:"url(blah de blah)"     }     ,unclickedStyle:{       backgroundImage:"url(blah de blah)"     }     ,elements:{     }   }   /// blah, blah, more like this };   // now lets make some buttons function makeFloatingButtons(){   var buttons=appData.floatingButtons;   function getButton(x){     var rv=buttons.elements['floatingButton'+x]=document.getElementById('floatingButton'+x);     return rv;   }   function styleButton(el){     var styles=buttons.style;     var o=el.style;     for (var i in styles) {//see how this is magic? just declare new styles when you need them       o[i]=styles[i];     }   }   for (var x=0;x<buttons.count;x+=1) {     var btn=getButton(x);     styleButton(btn);   } }  function clearFloatingButtons(){   var buttons=appData.floatingButtons;   //easy   for (var i in buttons.elements) {     var style=buttons.elements[i].style;     for (var j in buttons.clearStyle) {       style[j]=buttons.clearStyle[j];// more magic     }   } } 

Concentrate on the structure of your data, and genericizing your functions, then eventually you will learn how to move your functions into that global object and then start thinking in an OOP manner (once you see how functions and data CAN live like brother and sister IF YOU ARE ORGANIZED).

 
 

Relacionados problema

3  Función de estilo sortable jquery  ( Jquery sortable style function ) 
Esta es una función de clasificación básica escrita en jQuery que mueve los elementos en la DOM alrededor para crear espacios vacíos para un gotpable (). Como...

1  Enviando la página web actual a un sitio de marcadores  ( Submitting the current webpage to a bookmarking site ) 
Me gustaría escuchar sus comentarios sobre el siguiente enlace que permite a los usuarios agregar un enlace a un sitio web. Leí que join()5 es sugerido en...

5  Función de movimiento para un juego  ( Move function for a game ) 
Tengo una función PackageSubpackageSubpackageName6 en este juego que estoy haciendo. Funciona bien, pero me gustaría reducir un poco su tamaño. PackageSu...

10  "Stardust" Simulador 2D Gravity - Seguimiento 1: Los planetas  ( Stardust 2d gravity simulator follow up 1 the planets ) 
Este es un seguimiento para el juego de gravedad del simulador 2D pregunta. Desde entonces, he desarrollado mis habilidades de JavaScript, y ahora esto...

3  Página web basada en una muestra de un libro  ( Web page based on a sample from a book ) 
He creado una página web basada en una muestra de un libro. Funciona bien, pero parece haber sido demasiado complicado. dt4 ¿Es posible mejorar la clari...

3  Convertir un estado de hash a un objeto JavaScript  ( Converting a state from hash into a javascript object ) 
Este código obtiene un estado desde una parte de hash de una cadena de consulta, convierte la parte de hash a un objeto JavaScript y luego intenta obtener un ...

2  IMACROS BOT para realizar refrescos  ( Imacros bot for performing refreshes ) 
Estoy tratando de simplificar este código. Parece que todo funciona como debería; Sin embargo, cuando en el bucle de actualización de Imacro, parece un poco i...

7  Merge Sort en JavaScript  ( Merge sort in javascript ) 
Implementé este tipo de fusión en JS y noté que para los números de enteros aleatorios es mucho más rápido que la construcción en funciones de tipo de todos l...

7  Elementos de anidación con apendchild  ( Nesting elements with appendchild ) 
Quiero hacer un navegación que se vea así: <nav class="socialmediaicons"> <ul> <li> <a> <img> </a> </li> </ul> </nav> ...

2  Limpiando una galería de imágenes rotativas  ( Cleaning up a rotating image gallery ) 
He creado una pequeña galería de imágenes para la web. Me propuse querer 3 cosas. 1. Toda la galería tenía una capacidad de respuesta a nivel básica. 2. La ga...




© 2022 respuesta.top Reservados todos los derechos. Centro de preguntas y respuestas reservados todos los derechos