-
Notifications
You must be signed in to change notification settings - Fork 177
Create getDataDictionary.js #172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create getDataDictionary.js #172
Conversation
use this in client side like ui action get all active data from selected table.
|
@kumarinisha378 thank you for your submission. I have started reviewing your code, however, from the description it is not clear to me what your intend of this code is. Can you please add more context? |
WillemZeiler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@earlduque is this still relevant? It is an open pull request, but not in the 3 main projects.
| var dictionaryInfo = {}; | ||
|
|
||
| // Initialize field counts | ||
| var gr = new GlideRecord(tableNames); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Kumarinisha378,
thank you for submitting this! Appreciate the effort and extend of this new file. Please note gr as variable name should be avoided. Can you use descriptive variable names instead?
| var fields = gr.getFields(); | ||
| for (var i = 0; i < fields.size(); i++) { | ||
| var fieldName = fields.get(i).getName(); | ||
| fieldCounts[fieldName] = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would combine this. Instead of gliderecord through all records initializing, then going over them in another (same query) on line 17-29, it would be better to iterate through each record once and set the fieldCounts. This requires less lines of code and impacts the performance less.
| } | ||
|
|
||
| // Iterate through each record and count non-empty fields | ||
| gr = new GlideRecord(tableNames); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tableNames suggests it is plural, however, the code only accepts one table. Would be good to change the name. Additionally, provide a comment on the function and it's parameters. For example /** Returns x to the n-th power. @param {number} x The number, @param {number} n The power, @return {number} The result. */
Use this in client side like ui action get all active data from selected table.