3
\$\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>";
 }
 });
 }
 });
asked Nov 12, 2022 at 3:53
\$\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
 */
answered Nov 13, 2022 at 19:30
\$\endgroup\$
2
  • \$\begingroup\$ Thanks how would i use that code ? \$\endgroup\$ Commented 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\$ Commented Nov 14, 2022 at 2:14

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.