2
\$\begingroup\$

I am working on this project where I need to building html from this object. There a a lot of loops and counter and wanted to see if there is a better way to do this

Any suggestions/comments are greatly appreciated to improve the code further. Thanks.

full function https://jsfiddle.net/tjmcdevitt/xLbusdk3/10/

$.each(question.MeetingPollingParts, function (index, value) {
    if (value.MeetingPollingPartsTypeId === MeetingPollingPartsTypeId.Image) {

        $.each(value.MeetingPollingPartsValues, function (index, parts) {
            console.log(parts);
            HTML += "<div class='row'>";
            HTML += "<div class='col-md-12'>";
            HTML += "<div class='form-group'>";
            let strImage = "data:" + parts.FileType + ";base64," + parts.FileData;
            console.log(strImage);
            HTML += "<img src='" + strImage + "' />";
            HTML += "</div>";
            HTML += "</div>";
            HTML += "</div>";
        });
    }

    if (value.MeetingPollingPartsTypeId === MeetingPollingPartsTypeId.Answer) {

        $.each(value.MeetingPollingPartsValues, function (index, parts) {               
            if (parts.MeetingPollingPartsValuesTypeId === MeetingPollingPartsValuesTypeId.Radio) {
                htmlthTable += "<th>";
                htmlthTable += parts.QuestionValue;
                htmlthTable += "</th>";
                countRadio = countRadio + 1;
            }
        });

    }
    console.log(countRadio);
    if (value.MeetingPollingPartsTypeId === MeetingPollingPartsTypeId.RowQuestion) {
        $.each(value.MeetingPollingPartsValues, function (index, parts) {
            if (parts.MeetingPollingPartsValuesTypeId === MeetingPollingPartsValuesTypeId.MultipleChoiceGrid) {
                htmltrTable += "<tr><td>";
                htmltrTable += parts.QuestionValue;
                htmltrTable += "</td>";
                for (var i = 1; i <= countRadio; i++) {
                    htmltrTable += "<td>";
                    htmltrTable += "<input type='radio'>";
                    htmltrTable += "</td>";
                }
                htmltrTable += "</tr>";
            }
        });
    }
    });
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

The naming conventions are irregular; it is conventional in JS to use camelCase for variables, and Capitalized for class names, and ALLCAPS for constants such as static properties of classes, and perhaps even the “enums” you appear to have for the type id to name -maps (but which are absent from both here and the fiddle).

String concatenation of hypertext using innerHtml is subject to malicious content since it doesn’t do any sanity checks, unlike for example textContent. Code-wise, it brings mental burden to the reader when there are different branching paths concatenating to the same hypertext string variable. Consider using backtick strings which can use ${code} inside, making it more readable.

The variable names seem to follow the logic of tracking the object hierarchy. I’m not sure why you’d want to do that, as long as you keep the namespace scope as tight as possible (which is a natural consequence of using smaller functions, preferably with a single responsibility per function). This point extends all the way to naming database fields.

I would consider to first partition the parts array by type to make it more readable, avoiding the need to branch out in the same function. Documenting the different object hierarchies using JSDoc would also help the reader immensely, especially in case of any transformations. For example (as per above, I’ve shortened the property names below, the original names can be seen in the comments):

/** @typedef {Object} Question
 *  @property {Number}  id          - MeetingPollingQuestionId
 *  @property {?String} type        - MeetingPollingQuestionType
 *  @property {Number}  pollId      - MeetingPollingId
 *  @property {Number}  typeId      - MeetingPollingQuestionTypeId
 *  @property {Number}  order       - SequenceOrder
 *  @property {Array.<Part>} parts  - MeetingPollingParts
 *
 *  @typedef {Object} Part
 *  @property {Number}  id              - MeetingPollingPartsId
 *  @property {?String} type            - Type
 *  @property {Number}  typeId          - MeetingPollingPartsTypeId
 *  @property {Number}  questionId      - MeetingPollingQuestionId
 *  @property {Array.<Option>} options  - MeetingPollingPartsValues
 *
 *  @typedef {Object} Option
 *  @property {Number} id               - MeetingPollingPartsValuesId
 *  @property {Number} partId           - MeetingPollingPartsId
 *  @property {Number} typeId           - MeetingPollingPartsValuesTypeId
 *  @property {String} content          - QuestionValue
 *  @property {?Number} fileManagerId   - FileManagerId
 *  @property {?String} fileName        - FileName
 *  @property {?String} fileData        - FileData
 *  @property {?String} fileType        - FileType
 *  @property {?Array} images           - XXX MeetingPollingPartsValuesImages
 */
\$\endgroup\$
2
  • \$\begingroup\$ Thanks how would i use that code ? \$\endgroup\$
    – Jefferson
    Nov 14, 2022 at 0:20
  • 2
    \$\begingroup\$ @Jefferson Which code? The JSDoc fragment is just an instruction to other human beings about the form and relation of the data being used, although there are editors and IDEs which can use the information, and languages like typescript to use them as type definitions. The point I was making with it was also about property naming. \$\endgroup\$
    – morbusg
    Nov 14, 2022 at 2:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.