首页
学习
活动
专区
圈层
工具
发布
社区首页 >问答首页 >简单待办事项清单

简单待办事项清单
EN

Code Review用户
提问于 2015-03-26 22:36:07
回答 1查看 6.7K关注 0票数 6

有人能告诉我,这段代码对于一个由香草JavaScript制作的超级简单的待办事项列表有多好/有多坏?在我的下一个项目上有什么要做或考虑的吗?

代码语言:javascript
复制
// establish variables
var addButton = document.getElementById('btnAdd');

// event listeners
addButton.addEventListener("click", addItem);

// assign event listener to all delete links
function assignDeleteLinkEvent(){
    var deleteLinks = document.getElementsByClassName('delete-link');

    for (var i=0;i<deleteLinks.length;i++){
        deleteLinks[i].addEventListener("click", removeItem);
    }
}

function addItem(){
    // establish variables
    var textBox = document.getElementById('textBox');
    var list = document.getElementById('todoList');
    var listElement = document.createElement('li');
    var deleteLink = document.createElement('a');
    // append item to list
    list.appendChild(listElement);
    listElement.innerText = textBox.value;
    // append delete link to item
    deleteLink.setAttribute("href", "#");
    deleteLink.setAttribute("class", "delete-link");
    deleteLink.innerHTML = "x";
    listElement.appendChild(deleteLink);
    assignDeleteLinkEvent();
    // reset input box
    textBox.value = "";
}

// removes the item
function removeItem(){
    var parent = this.parentNode.parentNode;
    var child = this.parentNode

    parent.removeChild(child);
}
EN

回答 1

Code Review用户

回答已采纳

发布于 2015-03-26 23:35:43

  1. 不要污染全局名称空间。您有1个变量和3个函数,它们都进入全局命名空间。这是一个特别的问题,因为你有通用的名字。所以它们可能会在你不知道的情况下被覆盖,这会引起问题。解决这一问题的最简单方法是将所有代码包装在一个自动执行的函数中。(函数(){. //所有代码都在这里。)();就您的目的而言,这是可行的。另一个选项是将其封装到一个函数中并调用该函数。这将允许您在以后需要时提供参数。函数toDoList() {. // code } toDoList();
  2. 限制访问DOM。在您的addItem函数中,您每次都请求todoList和textbox的DOM。这些变量不会改变,因此可以在此函数之外设置这些变量。函数toDoList() { var list = document.getElementById('todoList');var addButton = document.getElementById('btnAdd');.函数addItem() {.list.appendChild(listElement);…}
  3. 将代码组合成函数来简化。您正在addItem函数中创建一个删除链接。这包括四个步骤。将所有这些都移动到一个单独的函数中,并调用它。这使您的addItem函数更简单,并集中了删除链接创建代码。函数createDeleteLink() { var deleteLink = document.createElement('a');deleteLink.setAttribute("href“、"#");deleteLink.setAttribute("class”、“删除-链接”);deleteLink.innerHTML = "x";返回deleteLink }函数addItem() {.listElement.appendChild(createDeleteLink());.}现在删除链接的创建本身就很容易看到,我们可以在创建时直接添加单击事件侦听器,而不是每次循环每次链接(实际上是将侦听器添加到已经有侦听器的项中。
  4. 不必要的评论。像“移除一个项”这样的注释在一个叫做"removeItem“的函数上面是无用的。事实上,这只会让人分心。限制注释,以描述棘手的代码或任何以后可能令人困惑的东西。
  5. 今后的考虑。如果要重用此代码,可以考虑将options对象作为主函数的参数。这可以包括诸如主列表的id、删除链接的文本、类名等.这将允许您在多个页面上使用相同的代码,并允许自定义。

以下是我以上的想法(第五条除外):

代码语言:javascript
复制
        function initToDoList() {

            var textBox = document.getElementById('textBox');
            var list = document.getElementById('todoList');
            var addButton = document.getElementById('btnAdd');

            addButton.addEventListener("click", addItem);

            function createDeleteLink() {

                var deleteLink = document.createElement('a');
                deleteLink.setAttribute("href", "#");
                deleteLink.setAttribute("class", "delete-link");
                deleteLink.innerHTML = "x";
                deleteLink.addEventListener("click", removeItem);
                return deleteLink;

            }

            function addItem() {

                var listElement = document.createElement('li');
                listElement.innerText = textBox.value;
                listElement.appendChild(createDeleteLink());

                list.appendChild(listElement);

                textBox.value = "";
            }

            function removeItem() {

                var parent = this.parentNode.parentNode;
                var child = this.parentNode

                parent.removeChild(child);

            }
        }

        initToDoList();
票数 5
EN
页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持
原文链接:

https://codereview.stackexchange.com/questions/85116

复制
相关文章

相似问题

领券
问题归档专栏文章快讯文章归档关键词归档开发者手册归档开发者手册 Section 归档