Форум Сайтостроение Веб-строительство

Покритикуйте говнокод

D
На сайте с 28.06.2008
Offline
1114
Dram
313

Учусь JS, вот такая задача.

Дан следующий массив с работниками: 

let employees = [
        {name: 'employee1', age: 30, salary: 400},
        {name: 'employee2', age: 31, salary: 500},
        {name: 'employee3', age: 32, salary: 600},
];

1. Выведите на экран каждого работника в своем теге li тега ul.

2.  Сделайте так, чтобы по клику li  появлялся инпут для редактирования этого поля.

Мое решение, есть серьезные ошибки, упущения?

    let employees = [
        {name: 'employee1', age: 30, salary: 400},
        {name: 'employee2', age: 31, salary: 500},
        {name: 'employee3', age: 32, salary: 600},
    ];
    let body = document.querySelector('body');
    let ul = document.createElement('ul');
    body.appendChild(ul);
    for (let i =0; i < employees.length; i++){
        let lii = document.createElement('li');
        lii.innerText = employees[i]['name'] + ' '+ employees[i]['age'] + ' '+ employees[i]['salary'];
        ul.appendChild(lii);
        function eventLI () {
            lii.addEventListener("click", function fn (){
                let input = document.createElement('input');
                input.value = this.innerText;
                this.innerText = '';
                this.appendChild(input);
                lii.removeEventListener("click", fn)
                input.addEventListener("blur", function (){
                    lii.innerText = input.value;
                    input.remove();
                    eventLI ();
                })
            })
        }
        eventLI ();
    }

C
На сайте с 10.10.2013
Offline
90
Сергей Третьяков
#1 
<html>
<head>
<title>js test</title>
<script>
window.onload = function(){
let employees = [
        {name: 'employee1', age: 30, salary: 400},
        {name: 'employee2', age: 31, salary: 500},
        {name: 'employee3', age: 32, salary: 600},
];
let body = document.querySelector('body');
let ul = document.createElement('ul');
document.getElementsByTagName('body')[0].appendChild(ul);
for (let i =0; i < employees.length; i++){
        let lii = document.createElement('li');
        lii.innerText = employees[i]['name'] + ' '+ employees[i]['age'] + ' '+ employees[i]['salary'];
        ul.appendChild(lii);
        function eventLI () {
                lii.addEventListener("click", function fn (){
                        let input = document.createElement('input');
                        input.value = this.innerText;
                        this.innerText = '';
                        this.appendChild(input);
                        lii.removeEventListener("click", fn)
                        input.addEventListener("blur", function (){
                                lii.innerText = input.value;
                                input.remove();
                                eventLI ();
                        })
                })
        }
        eventLI ();
}
}
</script>
</head>
<body>
</body>
</html>


Работает и славно 😀

Разработка и продвижение сайтов. Консультирую 5000 руб/час.
Amigo_9876
На сайте с 01.04.2009
Offline
328
Amigo_9876
#2 
employees.forEach(function(item, i, arr){
        $('.emp').append('<li>' + employees[i].name + ', ' + employees[i].age + ', '  + employees[i].salary + '</li>');
})

Только Jquery подключить надо. for i - это уже прошлый век.

D
На сайте с 28.06.2008
Offline
1114
Dram
#3
Amigo_9876 #:
Jquery

Я ведь чистый JS учу, так что критика не принимается ))

Snake800
На сайте с 02.02.2011
Offline
236
Snake800
#4
Шикарно 👍. eventLI я бы вынес из цикла в отдельный метод, в качестве параметра принимающий lii.
S
На сайте с 13.10.2014
Offline
171
silicoid
#5
Как уже сказали выше, код лучше побить на логически-разделенные методы, чтоб было проще в нем ковыряться. В остальном, вроде всё норм.

Авторизуйтесь или зарегистрируйтесь, чтобы оставить комментарий