https://netsuiteprofessionals.com logo
Join Slack
Powered by
not to spam but just sharing... put up some code f...
# suiteql
r
not to spam but just sharing... put up some code for a 'SuiteQL Saved Search' its very alpha but may help someone else as we are getting great use in production. I expect to expand with 'SuiteQL Reminders' next https://github.com/redfish-dev/NetsuiteSavedSQLSearch
b
you have very weird scoping on your code
r
please explain, I am new to netsuite
b
Copy code
define(function () {
  var feildDefRec;

  function showListView(
    serverWidget,
    context,
    query,
    sqlSearchID,
    SqlQuery,
    records,
    elapsedTime
  ) {
    feildDefRec = loadFieldDefinitions(query, sqlSearchID);
      // Truncated
  }

  function getFieldDef(name) {
    var fieldDef = null;

    feildDefRec.forEach(function (element) {
      if (
        element["custrecordsql_fieldname"].toUpperCase() == name.toUpperCase()
      )
        fieldDef = element;
    });
    return fieldDef;
  }
});
you initialize 
feildDefRec
in one function to use in another
it forces your functions to be run in a certain order
which is a maintainability concern, you have to remember that order
r
I appreciate the feedback and I am refactoring as I go but having a global scoped variable is just a shortcut for rapid dev.
showListView is a major entry point (top level function) . but I don't discount your feedback.
b
thats not the worse one either
its just the one on top
you both pass around modules like serverWidget as parameters and use modules like file globally
r
yes
its a design pattern seen around many times to prevent meathod signatures from being 10 parameters long
b
the usual pattern is to use an object
not a global variable
its not even needed in this particular case
r
NS doesn't have have the general include/import/require library inclusion making their code nasty if you have to pass 10-15 modules around. Do you have a recommendation ?
m
Why do you need to pass modules around?
b
SSSEngine.js could have just looked like
Copy code
define(['N/file'], function (file) {
it uses amd as a module loader
its weird serverside, but is a functional module system
r
battk, so can I just reference file 5 funtion calls deep without passing through ach?
each
b
correct
it uses function scope
r
weird becuase I get null ref
let me provide a quick sample. I would like to learn
interesting look at this link
Copy code
var 
     log,
     search,
     response = new Object();     


define( [ 'N/log', 'N/search' ], main );


function main( logModule, searchModule ) {

     log = logModule;
     search = searchModule;
m
not sure what the point of that is? why not just
Copy code
define(['N/log', 'N/search'], function(log, search) {
});
r
michoel, let me provide a sample in a few min
ok here is the problem
in SSSEngine
Copy code
function showQuery(serverWidget, context, query, sqlSearchID, displayType, logMod){
inside inner module
Copy code
function showListView(serverWidget,context, query, sqlSearchID, SqlQuery, records, elapsedTime) {

    feildDefRec = loadFieldDefinitions(query, sqlSearchID);
    logMod.debug('fieldRecs', feildDefRec);
org.mozilla.javascript.EcmaError: ReferenceError: "logMod" is not defined. (/SuiteScripts/Engine/SSSEngine.js#49)
but this is an JS file references and doesnt have the defines
b
Copy code
define(['N/log'], function (logMod) {
is what your define statement needs to look like at the top
m
You don't even need to explicitly require 
log
module. It's available globally. You can just use it as 
log.debug
b
it will work the same way as your suitelet entrypoint
👀 1
r
I will try this. Thank you for the lesson!
s
I'll not go in depth here to derail anything but perhaps worth mention that a lot of this code would be simpler with TypeScript (where you can use `import statements) and even more so with NFT.
r
I appreciate the feedback even if not what was expected
m
Copy code
var file, serverWidget, record, format, context, query, form, SSSEngine, log;

define(['N/file','N/ui/serverWidget','N/record',  'N/format',  'N/query', './../Engine/SSSEngine', 'N/log'],

function(fileModule, serverWidgetModule, recordModule, formatModule, queryModule, SSSEngineModule, logModule) {

    file = fileModule;
    serverWidget = serverWidgetModule;
    record = recordModule;
    format = formatModule;
    query = queryModule;
    SSSEngine = SSSEngineModule;
    log=logModule;
can be simplified to
Copy code
define(['N/file','N/ui/serverWidget','N/record', 'N/format', 'N/query', '../Engine/SSSEngine'], function(file, serverWidget, record, format, query, SSSEngine) {
r
Michoel I had similar earlier but ran into losing the object but let me try again
s
also @redfishdev are you familiar with @tdietrich’s Suitelet for SQL Queries? I wish folks would team up and work together on one product in this space
at least initially
Wouldn't it be swell if all were innovating to make one awesome SuiteQL query tool instead of several mediocre ones? I think the efforts I've seen so far are open source
r
stalbert, I have sent this to tim. my goal is to fuel him to build the great one so I can download : )
@michoel @stalbert thank you for the lesson... at least inside a suitelet log was globally scoped
I will try next a JS file inclusion
this worked
function(fileModule, serverWidgetModule, recordModule, formatModule, queryModule, SSSEngineModule, log) { function onRequest(contextMod) { InnerFunct(); } function InnerFunct() { log.debug('test', 'test'); } }
b
log is normally global
however, you overwrote the global in
Copy code
var file, serverWidget, record, format, context, query, form, SSSEngine, log;
r
do I even dare try to show you my advance calendar UI?
b
so its globally undefined
r
message has been deleted
or content rendering engine....
maybe I need to refactor code for a few days first
@battk I removed the global log var in my test
m
@redfishdev please take whatever we say as constructive feedback. speaking at least for myself but I hope for everyone else on channel the point isn't to criticize or nitpick your code
b
the N/log module is one of the few global modules in netsuite
your code will function the same using
Copy code
/**
 * @NApiVersion 2.x
 * @NScriptType Suitelet
 * @NModuleScope SameAccount
 */
define([], function () {
  function onRequest(contextMod) {
    InnerFunct();
  }
  function InnerFunct() {
    log.debug("test", "test");
  }
});
N/util is also global, but I consider it not worth the effort to mock while testing
you can also do fun but mostly useless things like
Copy code
/**
 * @NApiVersion 2.x
 * @NScriptType Suitelet
 * @NModuleScope SameAccount
 */
define(['N/log'], function (logModule) {
  function onRequest(contextMod) {
    InnerFunct();
  }
  function InnerFunct() {
    log.debug("test", "test");
    logModule.debug("second test", "second test");
  }
});
r
@michoel all good. I appreciate the knowledge and @battk provided some great points and taught me some things. Maybe I don't agree with only locally scoped variables and passing around long method signatures but I think it is rapid dev vs maintainability argument. This was written in around a day which is about all I have time for to respond to our startup business needs at that time. @battk thank you for the help and knowledge. I haven't used Util but will look into it. I would really welcome a Collab open source Sql Search engine that can be linked to grids, calendars, drill downs, content delivery via html and pdf and scheduled delivery. @tdietrich if you read this, let me know if you need someone to fast code inappropriately.
I am slowly building the knowledge to accomplish in NS with only being introduced to the platform in June and go live in Sept. But these forums help alot
b
the usual approach for long signatures is to use an object as a parameter for example,
Copy code
SSSEngine.showQuery({
  serverWidget: serverWidget,
  context: context,
  query: query,
  savedSQLSearchID: savedSQLSearchID,
  displayType: displayType,
  log: log,
});
instead of
Copy code
SSSEngine.showQuery(serverWidget, context, query,savedSQLSearchID, displayType,log);
r
how is that not a long signature?
are those then gloablly scoped?
b
they arent global, they would be functional scoped
r
its not about the entry meathod signature
its about the inner functions.
treating JS as a rich object when you can
even knowing that if someone called out of order can cause an issue. but I challenge anyone to look at the code and call a inner helper function before the main entry
I want to learn but this point seams like a purist point vs rapdi dev pattern
b
its not just a rapid thing
the reason i brought it up is that its hard to read and hard to reason about when you rely on global variables
for example. it makes it hard to tell if 
gridBaseHTMLFile
is a global defined somewhere else
or is a hardcoded artifact that you removed at some point
r
Ok back to my original point, you convinced me but I have an issue implementing
look at this sample
nevermind. Time for bed, thanks for discussion
b
your scoping is weird since you are using parameters when you could just declare your functions in the correct place
Copy code
/** *
 *
 * Saved Sql Search Engine
 *
 */

define(function () {
  var log;
  var feildDefRec;
  var columnsRec;

  function showQuery(
    serverWidget,
    context,
    query,
    sqlSearchID,
    displayType,
    logMod
  ) {
    function getFieldType(name) {}
    function formatFieldByType(name, val, row, record) {}
    function getColumnName(index) {}
    function getFeildRawValue(name, record) {}
    function getFieldDef(name) {}
    function loadBaseQuery(query, sqlSearchID) {}
    function loadFieldDefinitions(query, sqlSearchID) {}
    function buildLink(dashletID, ColumnName) {}

    var SqlQuery;
    log = logMod;

    SqlQuery = loadBaseQuery(query, sqlSearchID);

    //get rid of carriage returns
    SqlQuery = SqlQuery.replace(/(\n)/gm, " ");
    SqlQuery = SqlQuery.replace(/(\r)/gm, " ");
    SqlQuery = SqlQuery.replace(/(\t)/gm, " ");

    // Run the query.
    var queryResults = query.runSuiteQL({ query: SqlQuery });
    // Get the mapped results.
    var beginTime = new Date().getTime();
    var records = queryResults.asMappedResults();
    var endTime = new Date().getTime();
    var elapsedTime = endTime - beginTime;
      
    function showListView() {}
    function showHTMLGrid() {}

    if (displayType === "htmlgrid") {
      showHTMLGrid(SqlQuery, records, elapsedTime);
    } else if (displayType === "list") {
      showListView(SqlQuery, records, elapsedTime);
    }
  }

  return { showQuery: showQuery };
});
if you moved where you declare your functions, you wouldnt even need parameters
its the same problem made in different places and different ways
r
you brought me back
u make assumptions
u assume these functions arent shared
b
although its a vast and wrong simplification, inner functions share the other function's scope
r
and only scoped under the showQuery functino
this discounts the possibility of using a different UI ShowHTMLVersin() type functin
then each have to be written again
function getFieldType(name) {} function formatFieldByType(name, val, row, record) {} function getColumnName(index) {} function getFeildRawValue(name, record) {}
all are reusable with different Show() type function
ShowCalendar() for example
ShowAdvancedGrid()
ShowList()
why have inner local functions that are genral purpose?
"wrong simplification"
some lesson learned, some assuptions dont apply
b
same reason in the beginning, getFieldType is not actually reusable, you will have to remember to set 
feildDefRec
in your new show function
same thing for getColumnName
r
good night @battk thx for feedback.
3 Views
Open in Slack
PreviousNext
Powered by