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.

Sign up for GitHub

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

Jump to bottom

Trunk 2303 #266

Closed
wants to merge 5 commits into from
Closed

Trunk 2303 #266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9912509
TRUNK-2303: Fixed formatting problem regarding the return of doubles
ioyou Apr 3, 2013
a9984b5
TRUNK-2303: Fixed formatting problem regarding the return of doubles
ioyou Apr 3, 2013
9de77d9
TRUNK-2303: Fixed formatting problem regarding the return of doubles
ioyou Apr 3, 2013
b949fc5
TRUNK-2303: Fixed return of long decimal numbers to not use scientifi…
ioyou Apr 3, 2013
de564b9
TRUNK-2303: Fixed return of long decimal numbers to not use scientifi…
ioyou Apr 3, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 9 additions & 3 deletions api/src/main/java/org/openmrs/Obs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
package org.openmrs;

import java.text.DateFormat;
import java.text.NumberFormat; // new formatter for getValueAsString
Copy link
Member

Choose a reason for hiding this comment

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

No need to comment import. Please remove it.

import java.text.DecimalFormat;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Date;
Expand Down Expand Up @@ -916,8 +918,13 @@ public void setAccessionNumber(String accessionNumber) {
* @should return first part of valueComplex for non null valueComplexes
* @should return non precise values for NumericConcepts
* @should return proper DateFormat
* @should not return long decimal numbers as scientific notation
*/
public String getValueAsString(Locale locale) {
// formatting for the return of numbers of type double
NumberFormat nf = NumberFormat.getNumberInstance(locale);
DecimalFormat df = (DecimalFormat) nf;
df.applyPattern("#0.0"); // formatting style
//branch on hl7 abbreviations
if (getConcept() != null) {
String abbrev = getConcept().getDatatype().getHl7Abbreviation();
Expand Down Expand Up @@ -953,7 +960,7 @@ else if (abbrev.equals("CWE")) {
int i = (int) d;
return Integer.toString(i);
} else {
getValueNumeric().toString();
df.format(getValueNumeric());
}
}
}
Expand All @@ -977,7 +984,7 @@ else if (abbrev.equals("ED") && getValueComplex() != null) {

// if the datatype is 'unknown', default to just returning what is not null
if (getValueNumeric() != null)
return getValueNumeric().toString();
return df.format(getValueNumeric());
else if (getValueCoded() != null) {
if (getValueDrug() != null)
return getValueDrug().getFullName(locale);
Expand Down Expand Up @@ -1130,5 +1137,4 @@ public void setPreviousVersion(Obs previousVersion) {
public Boolean hasPreviousVersion() {
return getPreviousVersion() != null;
}

}
63 changes: 38 additions & 25 deletions api/src/test/java/org/openmrs/ObsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
import org.openmrs.test.Verifies;

/**
* This class tests all methods that are not getter or setters in the Obs java object TODO: finish
* this test class for Obs
* This class tests all methods that are not getter or setters in the Obs java
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you have the right code template, and this line was reformatted to be too short. Make sure you have the code formatter and template defined at https://wiki.openmrs.org/display/docs/Coding+Conventions

* object TODO: finish this test class for Obs
*
* @see Obs
*/
Expand All @@ -53,7 +53,8 @@ public void shouldAddandRemoveObsToGroup() throws Exception {
// These methods should not fail even with null attributes on the obs
assertFalse(obsGroup.isObsGrouping());
assertFalse(obsGroup.hasGroupMembers(false));
assertFalse(obsGroup.hasGroupMembers(true)); //Check both flags for false
assertFalse(obsGroup.hasGroupMembers(true)); // Check both flags for
Copy link
Member

Choose a reason for hiding this comment

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

same comment here about your line length being too short

// false

// adding an obs when the obs group has no other obs
// should not throw an error
Expand Down Expand Up @@ -102,23 +103,23 @@ public void shouldGetRelatedObservations() throws Exception {
o.setPerson(new Patient(2));
o.setValueText("childObs");

//create its sibling
// create its sibling
Obs oSibling = new Obs();
oSibling.setDateCreated(new Date());
oSibling.setLocation(new Location(1));
oSibling.setObsDatetime(new Date());
oSibling.setValueText("childObs2");
oSibling.setPerson(new Patient(2));

//create a parent Obs
// create a parent Obs
Obs oParent = new Obs();
oParent.setDateCreated(new Date());
oParent.setLocation(new Location(1));
oParent.setObsDatetime(new Date());
oSibling.setValueText("parentObs");
oParent.setPerson(new Patient(2));

//create a grandparent obs
// create a grandparent obs
Obs oGrandparent = new Obs();
oGrandparent.setDateCreated(new Date());
oGrandparent.setLocation(new Location(1));
Expand All @@ -130,7 +131,7 @@ public void shouldGetRelatedObservations() throws Exception {
oParent.addGroupMember(oSibling);
oGrandparent.addGroupMember(oParent);

//create a leaf observation at the grandparent level
// create a leaf observation at the grandparent level
Obs o2 = new Obs();
o2.setDateCreated(new Date());
o2.setLocation(new Location(1));
Expand All @@ -141,17 +142,18 @@ public void shouldGetRelatedObservations() throws Exception {
oGrandparent.addGroupMember(o2);

/**
* test to make sure that if the original child obs calls getRelatedObservations, it returns
* itself and its siblings: original obs is one of two groupMembers, so relatedObservations
* should return a size of set 2 then, make sure that if oParent calls
* getRelatedObservations, it returns its own children as well as the leaf obs attached to
* the grandparentObs oParent has two members, and one leaf ancestor -- so a set of size 3
* should be returned.
* test to make sure that if the original child obs calls
* getRelatedObservations, it returns itself and its siblings: original
* obs is one of two groupMembers, so relatedObservations should return
* a size of set 2 then, make sure that if oParent calls
* getRelatedObservations, it returns its own children as well as the
* leaf obs attached to the grandparentObs oParent has two members, and
* one leaf ancestor -- so a set of size 3 should be returned.
*/
assertEquals(o.getRelatedObservations().size(), 2);
assertEquals(oParent.getRelatedObservations().size(), 3);

// create a great-grandparent obs
// create a great-grandparent obs
Obs oGGP = new Obs();
oGGP.setDateCreated(new Date());
oGGP.setLocation(new Location(1));
Expand All @@ -160,7 +162,7 @@ public void shouldGetRelatedObservations() throws Exception {
oGGP.setValueText("grandParentObs");
oGGP.addGroupMember(oGrandparent);

//create a leaf great-grandparent obs
// create a leaf great-grandparent obs
Obs oGGPleaf = new Obs();
oGGPleaf.setDateCreated(new Date());
oGGPleaf.setLocation(new Location(1));
Expand All @@ -170,24 +172,25 @@ public void shouldGetRelatedObservations() throws Exception {
oGGP.addGroupMember(oGGPleaf);

/**
* now run the previous assertions again. this time there are two ancestor leaf obs, so the
* first assertion should still return a set of size 2, but the second assertion sould
* return a set of size 4.
* now run the previous assertions again. this time there are two
* ancestor leaf obs, so the first assertion should still return a set
* of size 2, but the second assertion sould return a set of size 4.
*/
assertEquals(o.getRelatedObservations().size(), 2);
assertEquals(oParent.getRelatedObservations().size(), 4);

//remove the grandparent leaf observation:
// remove the grandparent leaf observation:

oGrandparent.removeGroupMember(o2);

//now the there is only one ancestor leaf obs:
// now the there is only one ancestor leaf obs:
assertEquals(o.getRelatedObservations().size(), 2);
assertEquals(oParent.getRelatedObservations().size(), 3);

/**
* finally, test a non-obsGroup and non-member Obs to the function Obs o2 is now not
* connected to our heirarchy: an empty set should be returned:
* finally, test a non-obsGroup and non-member Obs to the function Obs
* o2 is now not connected to our heirarchy: an empty set should be
* returned:
*/

assertNotNull(o2.getRelatedObservations());
Expand Down Expand Up @@ -271,6 +274,15 @@ public void getValueAsString_shouldReturnNonPreciseValuesForNumericConcepts() th
Assert.assertEquals(str, obs.getValueAsString(Locale.US));
}

@Test
@Verifies(value = "should not return long decimal numbers as scientific notation", method = "getValueAsString(Locale)")
public void getValueAsString_shouldNotReturnLongDecimalNumbersAsScientificNotation() throws Exception {
Obs obs = new Obs();
obs.setValueNumeric(123456789.0);
String str = "123456789.0";
Assert.assertEquals(str, obs.getValueAsString(Locale.US));
}

@Test
@Verifies(value = "should return proper DateFormat", method = "getValueAsString()")
public void getValueAsString_shouldReturnProperDateFormat() throws Exception {
Expand Down Expand Up @@ -301,7 +313,8 @@ public void getValueAsBoolean_shouldReturnTrueForValue_numericConceptsIfValueIs1

/**
* @see Obs#getGroupMembers(boolean)
* @verifies Get all group members if passed true, and non-voided if passed false
* @verifies Get all group members if passed true, and non-voided if passed
* false
*/
@Test
public void getGroupMembers_shouldGetAllGroupMembersIfPassedTrueAndNonvoidedIfPassedFalse() throws Exception {
Expand All @@ -317,7 +330,7 @@ public void getGroupMembers_shouldGetAllGroupMembersIfPassedTrueAndNonvoidedIfPa
assertEquals("set of all members should have length of 3", 3, members.size());
members = parent.getGroupMembers(false);
assertEquals("set of non-voided should have length of 2", 2, members.size());
members = parent.getGroupMembers(); //should be same as false
members = parent.getGroupMembers(); // should be same as false
assertEquals("default should return non-voided with length of 2", 2, members.size());
}

Expand All @@ -330,7 +343,7 @@ public void hasGroupMembers_shouldReturnTrueIfThisObsHasGroupMembersBasedOnParam
Obs parent = new Obs(5);
Obs child = new Obs(33);
child.setVoided(true);
parent.addGroupMember(child); //Only contains 1 voided child
parent.addGroupMember(child); // Only contains 1 voided child
assertTrue("When checking for all members, should return true", parent.hasGroupMembers(true));
assertFalse("When checking for non-voided, should return false", parent.hasGroupMembers(false));
assertFalse("Default should check for non-voided", parent.hasGroupMembers());
Expand Down