Skip to content

show call time and instance id in bmi page.#246

Merged
seanyinx merged 6 commits intoapache:masterfrom
zhuhoudong:ServiceComb-Java-Chassis-zhu
Nov 8, 2017
Merged

show call time and instance id in bmi page.#246
seanyinx merged 6 commits intoapache:masterfrom
zhuhoudong:ServiceComb-Java-Chassis-zhu

Conversation

@zhuhoudong
Copy link
Copy Markdown
Contributor

update BMI samples. Show call time and instance id in bmi page.

Signed-off-by: zhuhoudong

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.007%) to 86.453% when pulling d2bc489 on zhuhoudong:ServiceComb-Java-Chassis-zhu into b18a893 on ServiceComb:master.

return roundToOnePrecision(bmi);

double result = roundToOnePrecision(bmi);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

apply code style first, the indent should be two spaces instead of four. You can find the code style file at the ServiceComb-Java-Chassis/etc directory.

After import the code style, reformat the code again.


double result = roundToOnePrecision(bmi);

MicroserviceInstance name = RegistryUtils.getMicroserviceInstance();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use instance as the variable name may be better than the name.

MicroserviceInstance name = RegistryUtils.getMicroserviceInstance();
String processID = name.getInstanceId().substring(0, 12);

Date date=new Date();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

leave space before and after the equal symbol. This will be done when you reformat the code after importing the code style.


Date date=new Date();
DateFormat format = new SimpleDateFormat("HH:mm:ss");
String calltime = format.format(date);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use camelCase for the variable name, namely callTime

Map<String, String> map = new HashMap<String, String>();
map.put("result", Double.toString(result));
map.put("processID", processID);
map.put("calltime", calltime);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same as above

DateFormat format = new SimpleDateFormat("HH:mm:ss");
String calltime = format.format(date);

Map<String, String> map = new HashMap<String, String>();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe resultMap is better

<div class="alert alert-light" role="alert" id="bmi">
<h3>Your BMI result is: <span id="bmi_result"></span></h3>
<h3>BMI Result: <span id="bmi_result"></span></h3>
<h3>BMI Container ID: <span id="bmi_processid"></span></h3>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

bmi_processId is better

<h3>Your BMI result is: <span id="bmi_result"></span></h3>
<h3>BMI Result: <span id="bmi_result"></span></h3>
<h3>BMI Container ID: <span id="bmi_processid"></span></h3>
<h3>BMI Called time: <span id="bmi_calltime"></span></h3>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

bmi_callTime is better

$("#bmi_result").text(data);
if ( data < 18.5 || (data < 30 && data >= 25) ) {
$("#bmi_result").text(data.result);
$("#bmi_processid").text(data.processID);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe change both processid and processID to processId is better

if ( data < 18.5 || (data < 30 && data >= 25) ) {
$("#bmi_result").text(data.result);
$("#bmi_processid").text(data.processID);
$("#bmi_calltime").text(data.calltime);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

change calltime to callTime is better

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.007%) to 86.439% when pulling bafdfb7 on zhuhoudong:ServiceComb-Java-Chassis-zhu into b18a893 on ServiceComb:master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.007%) to 86.439% when pulling bafdfb7 on zhuhoudong:ServiceComb-Java-Chassis-zhu into b18a893 on ServiceComb:master.

double heightInMeter = height / 100;
double bmi = weight / (heightInMeter * heightInMeter);
return roundToOnePrecision(bmi);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please create another interface to get the system information instead of hacking the old interface. BTW, it's not a good practice that using map for response message.

DateFormat format = new SimpleDateFormat("HH:mm:ss");
String callTime = format.format(date);

Map<String, String> resultMap = new HashMap<String, String>();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how can consumers know there are result/processId/callTime in response message?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can create a response class which holds the result , processId and callTime for the service consumer to use.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.006%) to 86.439% when pulling ff1aa97 on zhuhoudong:ServiceComb-Java-Chassis-zhu into b18a893 on ServiceComb:master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.04%) to 86.488% when pulling edda532 on zhuhoudong:ServiceComb-Java-Chassis-zhu into b18a893 on ServiceComb:master.

@seanyinx seanyinx merged commit 9f0e2fa into apache:master Nov 8, 2017
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.

6 participants