Skip to content

Rad 5888 meta script to use maintenance event service #22

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

RamKannekanti
Copy link

No description provided.

RAD-5888-Example meta script to use maintenance event service
RAD-5888-Create meta-script-to-use-maintenance-event-service.js
@RamKannekanti
Copy link
Author

Please review the file : meta_points_scripts/meta-script-to-use-maintenance-event-service.js
please neglect the other file Meta-MaintenanceEvent-Script.js

@RamKannekanti
Copy link
Author

Please review the file : meta_points_scripts/meta-script-to-use-maintenance-event-service.js

please neglect the other file : Meta-MaintenanceEvent-Script.js

Copy link
Contributor

@aasbra aasbra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested changes to improve readability, especially for users who are not developers.

var eventModelClass = new maintenanceEventModel();

//RQL related libraries
var ASTNode = Java.type('net.jazdw.rql.parser.ASTNode');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid using the same variable names in multiple scopes within the same script. While this code will execute correctly, a user who is not a developer may not understand the concept of variable shadowing and this can be confusing to them.

There are several variable names that are used multiple times including:

  • ASTNode
  • RQLUtils
  • eventInstance
  • eventInstanceService
  • sumOfActiveMaintenanceEventsCount

//function to return the active MaintenanceEvent count linked to pointXids
function getPointXidActiveMaintenanceEvents(pointXid) {

var sumOfActiveMaintenanceEventsCount = new ArrayList();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable name is a little confusing. This variable represents a list of items, not a numerical sum. Choosing a different variable name would improve readability.

//first check whether this Meta data point has any acitve maintenance events, if has return 0
var metapointActiveMtcEventsCount = 0;
var pointValueSum = 0;
var MetaPointXid = 'DP_a620e9ab-8760-4177-8c1a-fecea969652f'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This XID doesn't have to be linked to a Meta Point, it can be used for any data point. This variable name should be renamed to avoid confusing users.

var metapointActiveMtcEventsCount = 0;
var pointValueSum = 0;
var MetaPointXid = 'DP_a620e9ab-8760-4177-8c1a-fecea969652f'
var isMetaPointEnabled = RuntimeManager.isDataPointEnabled(MetaPointXid);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This XID doesn't have to be linked to a Meta Point, it can be used for any data point. This variable name should be renamed to avoid confusing users.

var eventInstanceServiceClass = Common.getBean(EventInstanceService.class);

//first check whether this Meta data point has any acitve maintenance events, if has return 0
var metapointActiveMtcEventsCount = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This XID doesn't have to be linked to a Meta Point, it can be used for any data point. This variable name should be renamed to avoid confusing users.

if (activeContextPointCount > 0) {
return pointValueSum;
} else {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a return value here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants