4

The loop below builds an HTML string using the object literal obj_fav, which holds a list of favorites.

Because the HTML string can be appended in multiple places, ns_tag namespaces the DOM ids to make sure they are unique.

This is somewhat besides the point. I feel the loop has poor logic. It works fine, but I find it hard to read and feel that is is poorly written. But I can't put my finger on why that is.

// $A.someIndex() implements array.prototype.some

// create favorite pane strings
$A.someIndex(obj_fav, function (val) {
    previous_id = current_id;
    current_id = this.A.ns_tag + selector + val.tag;

    // new tag found, close previous and open new
    if ((previous_id === undefined) || (previous_id !== current_id)) {

        // populate tag array
        this.A['tag_array' + selector].push(current_id);
        tag_string = tag_string + this.composeTag(current_id, selector);
        if (previous_id !== undefined) {
            favorite_string += '</div>';
        }
        favorite_string = favorite_string +
            '<div class="bookmark_page toggle_display_none" id = "' + current_id + '_page">';
    }

    // add set of favorites between page tags

    favorite_string += this.composeFavorite(val, selector);
}, this);

this snippet is contained in this klass:

/*******************************************************************************
**SArcmarks
*/


    var SArcmarks = $A.klass({

        Name: 'SArcmarks',

        // namespacing helps to prevent name clashes        // base_location_id_subdivisions        // there are 2 prefixes and one suffix

        A: {
            // namespace suffix             base:                    'udt'          loc:                     null,
            ns_tag:                  'udt_',
            ns_tag1:                 'udt_1_',
            tag_array:               [],
            tag_array1:              [],
            prev_page_el:            null,
            prev_tag_el:             null,
            but_text:                null,

            // toggles
            toggle_display_none:     'toggle_display_none',
            toggle_display_inline:   'toggle_display_inline',
            toggle_tag_active:       'toggle_tag_active',
            toggle_tag_lazy:         'toggle_tag_lazy',

            // morphing
            class_a:                 null
        },

        E: {
            hold_arcmarks:           '#hold_arcmarks',
            hold_tags:               '#hold_tags',
            aba_but_del:             '#aba_but_del',
            bookmark_link:           '@bookmark_link'
        },

/******************************************************************************/

        update: function (obj_fav, fav_el, tag_el) {
            var favorite_string = '',
                tag_string = '',
                current_id,
                previous_id,
                selector;

            if (fav_el) {
                selector = 1;
            } else {
                selector = '';
            }

            // create favorite pane strings
            $A.someIndex(obj_fav, function (val) {
                previous_id = current_id;
                current_id = this.A.ns_tag + selector + val.tag;

                // new tag found, close previous and open new
                if ((previous_id === undefined) || (previous_id !== current_id)) {

                    // populate tag array
                    this.A['tag_array' + selector].push(current_id);
                    tag_string = tag_string + this.composeTag(current_id, selector);
                    if (previous_id !== undefined) {
                        favorite_string += '</div>';
                    }
                    favorite_string = favorite_string +
                        '<div class="bookmark_page toggle_display_none" id = "' + current_id + '_page">';
                }

                // add set of favorites between page tags

                favorite_string += this.composeFavorite(val, selector);
            }, this);

            // close last tag

            favorite_string = favorite_string + '</div>';

            // if atleast one favorite/tag

            if (this.A['tag_array' + selector][0]) {
                if (!fav_el) {
                    this.E.hold_arcmarks.innerHTML = favorite_string;
                } else {
                    fav_el.innerHTML = favorite_string;
                }
                if (!tag_el) {
                    this.E.hold_tags.innerHTML = tag_string;
                } else {
                    tag_el.innerHTML = tag_string;
                }
                this.initPane(selector);
                this.flip($A.el('#' + this.A['tag_array' + selector][0]));
            }
        },

/******************************************************************************/

        composeFavorite: function (val, selector) {

            // add selector
            if (val.favicon === null) {
                val.favicon = 'arcmarks/images/image_null_50.png';
            }
            return '<div class = "am_hold" id = "' + val.id + selector + '">' +
                     '<img name="bo_im" id="' + $A.addU(val.id + selector, 'a') +
                     '" class="bookmark_image" src="' + val.favicon + '">' +
                     '<a target="_blank" name="bookmark_link" class="bookmark_link" href = "' +
                      val.url + '" id="' + $A.addU(val.id + selector, 'b') + '">' + val.title + '</a>' +
                   '</div>';
        },

/******************************************************************************/

        composeTag: function (current_id, selector) {

            // selector
            return '<p class="single_tag toggle_tag_lazy" id = "' + current_id + '">' +
                    current_id.slice(this.A['ns_tag' + selector].length) + '</p>';
        },

/******************************************************************************/

        // add listeners to tag(<p>) elements
        initPane: function (selector) {
            $A.someIndex(this.A['tag_array' + selector], function (val) {
                var self = this;
                $A.el('#' + val).addEventListener("click", function () {
                    self.flip(this);
                });
            }, this);
        },

/******************************************************************************/

        // flip page(<div>) | tag(<p>) given a tag

        flip: function (tag_element) {
            var page_element = $A.el('#' + tag_element.id + '_page');
            $A.addClass(page_element, this.A.toggle_display_inline);
            $A.addClass(tag_element, this.A.toggle_tag_active);
            if (this.A.prev_page_el && (tag_element.id !== this.A.prev_tag_el.id)) {
                $A.addClass(this.A.prev_page_el, this.A.toggle_display_none);
                $A.addClass(this.A.prev_tag_el, this.A.toggle_tag_lazy);
            }
            this.A.prev_page_el = page_element;
            this.A.prev_tag_el = tag_element;
            $A.log(page_element);
        },

        // insert favorite(<div>) given user input

        insertFavorite: function (obj_fav) {
            var tag_id = this.A.ns_tag + obj_fav.tag,
                div_el,
                html_string = this.composeFavorite(obj_fav),
                page_el = $A.el('#' + tag_id + '_page');

            div_el = $A.HTMLToElement(html_string);

            if (!page_el) {
                page_el = this.insertTagAndPage(tag_id);
            }
            $A.eachChild(page_el, function (iter) {
                if (iter === null) {
                    page_el.appendChild(div_el);
                    return true;
                }
                if (div_el.id < iter.id) {
                    page_el.insertBefore(div_el, iter);
                    return true;
                }
                if (iter === page_el.lastChild) {
                    page_el.appendChild(div_el);
                    return true;
                }
            });

            this.flip($A.el('#' + tag_id));
            return div_el;

        },

/******************************************************************************/

        // insert page(<div>) | tag(<p>) given a tag id

        insertTagAndPage: function (tag) {
            var par_el,
                div_el,
                hold_tags_el,
                hold_arcmarks_el,
                self = this;

            hold_tags_el = this.E.hold_tags;
            hold_arcmarks_el = this.E.hold_arcmarks;

            par_el = $A.createElement('p');
            par_el.innerHTML = tag.slice(this.A.ns_tag.length);
            par_el.className = "single_tag";
            par_el.id = tag;

            // insert the tag(<p>)

            $A.eachChild(hold_tags_el, function (iter) {
                if (iter === null) {
                    hold_tags_el.appendChild(par_el);
                    return true;
                }
                if (par_el.id < iter.id) {
                    hold_tags_el.insertBefore(par_el, iter);
                    return true;
                }
                if (iter === hold_tags_el.lastChild) {
                    hold_tags_el.appendChild(par_el);
                    return true;
                }
            });

            par_el.addEventListener("click", function () {
                self.flip(this);
            });

            div_el = $A.createElement('div');
            div_el.className = "bookmark_page";
            div_el.id = tag + "_page";
            div_el.style.display = "";

            // insert the page(<div>);

            $A.eachChild(hold_arcmarks_el, function (iter) {
                if (iter === null) {
                    hold_arcmarks_el.appendChild(div_el);
                    return true;
                }
                if (div_el.id < iter.id) {
                    hold_arcmarks_el.insertBefore(div_el, iter);
                    return true;
                }
                if (iter === hold_arcmarks_el.lastChild) {
                    hold_arcmarks_el.appendChild(div_el);
                    return true;
                }
            });

            return div_el;
        },

/******************************************************************************/

        // delete a favorite(<div>) given a link

        deleteFavorite: function (link_el) {
            var self = this,
                div_el = link_el.parentNode;
            $(div_el).toggle("explode", function () {
                if (div_el.previousSibling === null &&
                        div_el.nextSibling === null) {
                    self.deleteTagAndPage(div_el);
                }
                $A.removeElement(div_el);
            });
        },

/******************************************************************************/

        // delete tag(<p>) and page(<div>) given a favorite sub-element

        deleteTagAndPage : function (div_el) {
            var page_el = div_el.parentNode,
                tag_el = $A.el('#' + page_el.id.slice(0, -5)),
                hold_el = $A.el("#hold_tags");
            $A.removeElement(tag_el);
            $A.removeElement(page_el);
            $A.eachChild(hold_el, function (iter_el) {
                if (iter_el) {
                    this.flip(iter_el);
                    return true;
                }
            }, this);
        },

/******************************************************************************/

        morph: function (callback) {
            var i = 0,
                button = this.E.aba_but_del;
            if (this.A.but_text === 'Done') {
                this.A.but_text = 'Delete';
                this.A.class_a = 'bookmark_link';
                while (this.E.bookmark_link[i]) {
                    this.E.bookmark_link[i].removeEventListener("click", callback);
                    this.E.bookmark_link[i].className = this.A.class_a;
                    i += 1;
                }
                button.innerHTML = this.A.but_text;
            } else {
                this.A.but_text = 'Done';
                this.A.class_a = 'bookmark_delete';
                while (this.E.bookmark_link[i]) {
                    this.E.bookmark_link[i].addEventListener("click", callback);
                    this.E.bookmark_link[i].className = this.A.class_a;
                    i += 1;
                }
                button.innerHTML = this.A.but_text;
            }
        }

    }, true);
12
  • 3
    Can you briefly summarize what the loop is supposed to do? Anecdotally, I feel that this can be solved simply with a well-placed bit of jQuery code. Dec 9, 2013 at 23:21
  • It builds an HTML string from an object literal to be appended to the DOM. Dec 9, 2013 at 23:22
  • 2
    Yes, but what's the input (the relevant/SSCCE HTML) and what's the expected/desired output..? Dec 9, 2013 at 23:23
  • 1
    If it does what it is supposed to do, there are no performance problems, and it is unlikely to ever require maintenance, then it's not broken and you don't need to fix it. Dec 9, 2013 at 23:36
  • 2
    Wouldn't the code review site be more appropriate for this question? Dec 9, 2013 at 23:41

0

Browse other questions tagged or ask your own question.