Refaktoryzacja - funkcja Javascript

0
function getAlphabet() {
    div_alphabet_content = "";
    table_letters = new Array
    (
        "A", "Ą", "B", "C", "Ć", 
        "D", "E", "Ę", "F", "G", 
        "H", "I", "J", "K", "L", 
        "Ł", "M", "N", "Ń", "O", 
        "Ó", "P", "Q", "R", "S", 
        "Ś", "T", "U", "V", "W", 
        "X", "Y", "Z", "Ż", "Ź"
    );
    /*tablica liter 7x5*/
    var number_letter = "";
    for(i=0; i<=34; i++) {
        number_letter = "lett" + i;
        
        div_alphabet_content = div_alphabet_content + '<div class="letter" onclick="gameInferface('+i+')" id="'+ number_letter + '">' 
        + table_letters[i] + '</div>' + '\n'; // zmienna zawiera ciąg znaków !
        if ((i+1) % 7 === 0) 
        {
            div_alphabet_content = div_alphabet_content 
            + '<div id="corector"></div>\n';
        }
    }
    
    alphabet.innerHTML = div_alphabet_content;

Starzy wyjadacze domyślą się o co chodzi, potrzebuje ulepszyć kod. Chciałbym generować treść div'a za pomocą funkcji by było bardziej elegancko :)

A tu cd. fragment

String.prototype.setChar = function(position, char) {
    if (position > this.length - 1) {
        return this.toString();
    } else {
        return this.substr(0, position) + char + this.substr(position + 1);
    }
};
function refreshPassword()
{
    board.innerHTML = secret;
}
function gameInferface(arg) {
    hit = false;
    for(var i=0; i<pass_length; i++)
    {
        if (password.charAt(i) === table_letters[arg]) 
        {
            secret = secret.setChar(i, table_letters[arg]);
            hit = true;
        }
    }
1

Moja propozycja częściowego refaktora przy "generowaniu htmla" https://jsfiddle.net/x75ffq2m/

1
function fillWithAlphabet() {
    var content = ""; // var!!!
    var letters = new Array( // var!!!
        "A", "Ą", "B", "C", "Ć", 
        "D", "E", "Ę", "F", "G", 
        "H", "I", "J", "K", "L", 
        "Ł", "M", "N", "Ń", "O", 
        "Ó", "P", "Q", "R", "S", 
        "Ś", "T", "U", "V", "W", 
        "X", "Y", "Z", "Ź", "Ż" // ź przed ż
    );

    for (var i=0; i < letters.length; i++) { // var kurna!!!
        var letter = letters[i];
        content += '<div class="letter" data-letter="'+ letter + '">' + letter + '</div>';

        if ((i+1) % 7 === 0) 
            content += '<div class="corrector"></div>'; // id musi być unikalne. użyj class zamiast id. corRector a nie corector
    }
 
    alphabet.innerHTML = content;
}

// użyję jQuery, bo tak najłatwiej
$(".letter").click(function() { var currentLetter = $(this).data("letter"); alert(currentLetter); });

Zamiast tych elementów, które skatowałeś identycznym id "corector", powinieneś użyć kontenerów. W jQuery widzę to tak:

var $container = $(alphabet);
var $parent = $("<div class='container'>").appendTo($container);
for (var i=0; i < letters.length; i++) {
    var letter = letters[i];
    parent.append($('<div class="letter" data-letter="'+ letter + '">' + letter + '</div>'));

    if ((i+1) % 7 === 0) 
        $parent = $("<div class='container'>").appendTo($container);
}

oczywiście wartoby pozbyć się DRY na $parent = ...

2

W tym konkretnym przypadku dużo lepszym rozwiązaniem byłoby ręczne napisanie htmla - dane są statyczne, wszystkiego kilkanaście linijek - dłużej zajmuje napisanie funkcji renderującej, dodatkowo wydłużasz czas renderowania strony. Ok, ale skoro już musisz:

var letters = 'AĄBCĆDEĘFGHIJKLŁMNŃOÓPQRSŚTUVWXYZŹŻ';

function generateLettersHTML(letters, rowSize) {
    return letters
        .split('')
        .map(function(letter, index) {
            var corrector = !!rowSize && (index + 1) % rowSize === 0 ? '<hr class="corrector">\n' : '';
            return '<div class="letter" data-id=' + index + '>' + letter + '</div>\n' + corrector;
        })
        .join('');
}

console.log(generateLettersHTML(letters, 6));

ES6:

const letters = 'AĄBCĆDEĘFGHIJKLŁMNŃOÓPQRSŚTUVWXYZŹŻ';

const generateLettersHTML = (letters, rowSize) =>
    letters
        .split('')
        .map((letter, index) => {
            let corrector = !!rowSize && (index + 1) % rowSize === 0 ? '<hr class="corrector">\n' : '';
            return `<div class="letter" data-id=${index}>${letter}</div>\n${corrector}`;
        })
        .join('');

console.log(generateLettersHTML(letters, 6));

Dane wyciągnąłem poza funkcję by inne funkcje także mogły z nich korzystać, a sama funkcja była uniwersalna. Funkcja robi tylko jedną rzecz - generuje string z htmlem bez żadnych efektów ubocznych - nie ma tu żadnych globali i innych niespodzianek. Pomijam wrzucenie wyniku w DOM i przypisanie zdarzeń - w każdym razie najlepiej wyciągnąć to poza funkcję generującą.

Co do Twojego kodu:

  1. Wszechobecne globale - nie wiem czy celowo czy przypadkiem, w każdym razie jedno złe a drugie jeszcze gorsze.
  2. String.prototype.setChar = function(position, char) {...) - monkey patching to bardzo zła praktyka, nie rób tego,
  3. <div class="letter" onclick="gameInferface('+i+')> - nie używaj takiej obsługi zdarzeń, korzystaj z event listenerów.

PS
Trzymaj się z daleka od kursów Zelenta, są tragiczne (bo wygląda mi to na jego grę w "wisielca").

0

Dobra prawie mi się udało, no ale czemu funkcja wywołuje się dwa razy?

function getAlphabet() {
    table_letters = new Array
    (
        "A", "Ą", "B", "C", "Ć", 
        "D", "E", "Ę", "F", "G", 
        "H", "I", "J", "K", "L", 
        "Ł", "M", "N", "Ń", "O", 
        "Ó", "P", "Q", "R", "S", 
        "Ś", "T", "U", "V", "W", 
        "X", "Y", "Z", "Ż", "Ź"
    );
    var number_letter = "";
    for(var i=0; i<=34; i++) {
        number_letter = "lett" + i;
        var alphabet_div = new Array(34);
        alphabet_div[i] = document.createElement("div");
        textNode = document.createTextNode(table_letters[i]);
        alphabet_div[i].setAttribute("id", number_letter);
        alphabet_div[i].setAttribute("class", "letter");
        alphabet_div[i].setAttribute("onclick", "gameInferface("+i+")");

        alphabet_div[i].appendChild(textNode);
        alphabet.appendChild(alphabet_div[i]);
        if ((i+1) % 7 === 0) 
        {
            var corector = document.createElement("div");
            corector.setAttribute("id", "corector");
            alphabet.appendChild(corector);
            
        }
    }   
}
0

Nadal masz pełno globali, mimo, że zarówno @ŁF jak i ja o tym pisaliśmy. Masz właśnie idealny przykład dlaczego globale to zło - nikt nie jest Ci w stanie teraz ze 100% pewnością powiedzieć co jest źle, bo nie wiadomo co czym jest i jaka ma wartość bez patrzenia poza funkcję. No i pytasz czemu funkcja wywołuje się dwa razy, a nie pokazałeś nigdzie w jaki sposób ją wywołujesz.

1 użytkowników online, w tym zalogowanych: 0, gości: 1