Skip to content
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

rtRemote - Additional code cleanup #119

Closed
jmgasper opened this issue Feb 6, 2018 · 0 comments
Closed

rtRemote - Additional code cleanup #119

jmgasper opened this issue Feb 6, 2018 · 0 comments

Comments

@jmgasper
Copy link
Collaborator

jmgasper commented Feb 6, 2018

Additional code quality items from the recent review:

Issue 1:

Logger should be final:

  private static Logger logger = Logger.getLogger(...);

Issue 2:

The object maps should be final, they're not reinitialized:

  /**
  * the static map used to store local object, so that getObject can found the real local object
  */
  private static Map<string rtlocalobject> objectMap
      = new ConcurrentHashMap();
  /**
  * the remote object map, thread safe
  */
  private static Map<string uri> objectMap = new ConcurrentHashMap();

Issue 3:

Why do you use "this" to qualify objName but not value which is also an instance variable:

      this.objName = "obj://" + UUID.randomUUID().toString();
      value = buildValue();

Issue 4:

Should be private, not used outside class:

  public JsonObject buildValue() {
    JsonObjectBuilder builder = Json.createObjectBuilder();
    builder.add("object.id", this.objName);
    return builder.build();
  }
public void sendCallResponse(String correlationKey) throws RTException {
public void setListener(RTFunctionListener listener) {

Issue 5:

Should define a constant for property name "object.id":

    String objName = value.getString("object.id");
    builder.add("object.id", this.objName);

Should define a constant for property name "function.name":

    rtFunction.setFuncName(jsonObject.getString("function.name"));
    builder.add("function.name", funcName);

Issue 6:

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

No branches or pull requests

1 participant