openmrs · Apr 5, 2013 · Nov 4, 2013
Showing with 117 additions and 33 deletions.

+49 −22 api/src/main/java/org/openmrs/Obs.java

+68 −11 api/src/test/java/org/openmrs/ObsTest.java
diff --git a/api/src/main/java/org/openmrs/Obs.java b/api/src/main/java/org/openmrs/Obs.java
@@ -14,6 +14,8 @@
 package org.openmrs;
 
 import java.text.DateFormat;
+import java.text.DecimalFormat;
+import java.text.NumberFormat;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
 import java.util.Date;
@@ -45,21 +47,27 @@
  * Observations are collected and grouped together into one Encounter (one visit). Obs can be
  * grouped in a hierarchical fashion. <br/>
  * <br/>
+ * <p>
  * The {@link #getObsGroup()} method returns an optional parent. That parent object is also an Obs.
  * The parent Obs object knows about its child objects through the {@link #getGroupMembers()}
- * method. (Multi-level hierarchies are achieved by an Obs parent object being a member of another
- * Obs (grand)parent object) Read up on the obs table: http://openmrs.org/wiki/Obs_Table_Primer
+ * method.
+ * </p>
+ * <p>
+ * (Multi-level hierarchies are achieved by an Obs parent object being a member of another Obs
+ * (grand)parent object) Read up on the obs table: http://openmrs.org/wiki/Obs_Table_Primer In an
+ * OpenMRS installation, there may be an occasion need to change an Obs.
+ * </p>
+ * <p>
+ * For example, a site may decide to replace a concept in the dictionary with a more specific set of
+ * concepts. An observation is part of the official record of an encounter. There may be legal,
+ * ethical, and auditing consequences from altering a record. It is recommended that you create a
+ * new Obs and void the old one:
+ * </p>
+ * Obs newObs = Obs.newInstance(oldObs); //copies values from oldObs
+ * newObs.setPreviousVersion(oldObs);
+ * Context.getObsService().saveObs(newObs,"Your reason for the change here");
+ * Context.getObsService().voidObs(oldObs, "Your reason for the change here");
  * 
- * In an OpenMRS installation, there may be an occasion need to change an Obs. 
- * For example, a site may decide to replace a concept in the dictionary with a more specific
- * set of concepts. An observation is part of the official record of an encounter. There may 
- * be legal, ethical, and auditing consequences from altering a record. It is recommended
- * that you create a new Obs and void the old one:
- *      Obs newObs = Obs.newInstance(oldObs); //copies values from oldObs
- *      newObs.setPreviousVersion(oldObs);
- *      Context.getObsService().saveObs(newObs,"Your reason for the change here");
- *      Context.getObsService().voidObs(oldObs, "Your reason for the change here");
- *
  * @see Encounter
  */
 public class Obs extends BaseOpenmrsData implements java.io.Serializable {
@@ -161,8 +169,8 @@ public Obs(Integer obsId) {
 	 * @return a new Obs object with all the same attributes as the given obs
 	 */
 	public static Obs newInstance(Obs obsToCopy) {
-		Obs newObs = new Obs(obsToCopy.getPerson(), obsToCopy.getConcept(), obsToCopy.getObsDatetime(), obsToCopy
-		        .getLocation());
+		Obs newObs = new Obs(obsToCopy.getPerson(), obsToCopy.getConcept(), obsToCopy.getObsDatetime(),
+		        obsToCopy.getLocation());
 
 		newObs.setObsGroup(obsToCopy.getObsGroup());
 		newObs.setAccessionNumber(obsToCopy.getAccessionNumber());
@@ -345,10 +353,10 @@ public void setObsGroup(Obs obsGroup) {
 	}
 
 	/**
-	 * Convenience method that checks for if this obs has 1 or more group members (either voided or non-voided)
-	 * Note this method differs from hasGroupMembers(), as that method excludes voided obs; logic is that
-	 * while a obs that has only voided group members should be seen as "having no group members" it
-	 * still should be considered an "obs grouping"
+	 * Convenience method that checks for if this obs has 1 or more group members (either voided or
+	 * non-voided) Note this method differs from hasGroupMembers(), as that method excludes voided
+	 * obs; logic is that while a obs that has only voided group members should be seen as
+	 * "having no group members" it still should be considered an "obs grouping"
 	 * <p>
 	 * NOTE: This method could also be called "isObsGroup" for a little less confusion on names.
 	 * However, jstl in a web layer (or any psuedo-getter) access isn't good with both an
@@ -877,6 +885,15 @@ public void setComplexData(ComplexData complexData) {
 	 * 
 	 * <pre>
 	 * 
+	 * 
+	 * 
+	 * 
+	 * 
+	 * 
+	 * 
+	 * 
+	 * 
+	 * 
 	 * Obs obsWithComplexData = Context.getObsService().getComplexObs(obsId, OpenmrsConstants.RAW_VIEW);
 	 * </pre>
 	 * 
@@ -916,8 +933,17 @@ 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
+	 * @should use commas or decimal places depending on locale
+	 * @should not use thousand separator
+	 * @should return regular number for size of zero to or greater than ten digits
+	 * @should return regular number if decimal places are as high as six
 	 */
 	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 up to 6 digits
 		//branch on hl7 abbreviations
 		if (getConcept() != null) {
 			String abbrev = getConcept().getDatatype().getHl7Abbreviation();
@@ -953,7 +979,7 @@ else if (abbrev.equals("CWE")) {
 							int i = (int) d;
 							return Integer.toString(i);
 						} else {
-							getValueNumeric().toString();
+							df.format(getValueNumeric());
 						}
 					}
 				}
@@ -977,7 +1003,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);
@@ -1112,15 +1138,16 @@ public void setId(Integer id) {
 
 	/**
 	 * When ObsService updates an obs, it voids the old version, creates a new Obs with the updates,
-	 * and adds a reference to the previousVersion in the new Obs. 
-	 * getPreviousVersion returns the last version of this Obs. 
+	 * and adds a reference to the previousVersion in the new Obs. getPreviousVersion returns the
+	 * last version of this Obs.
 	 */
 	public Obs getPreviousVersion() {
 		return previousVersion;
 	}
 
 	/**
 	 * A previousVersion indicates that this Obs replaces an earlier one.
+	 * 
 	 * @param previousVersion the Obs that this Obs superceeds
 	 */
 	public void setPreviousVersion(Obs previousVersion) {

diff --git a/api/src/test/java/org/openmrs/ObsTest.java b/api/src/test/java/org/openmrs/ObsTest.java
@@ -53,7 +53,7 @@ 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 false
 
 		// adding an obs when the obs group has no other obs
 		// should not throw an error
@@ -102,23 +102,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));
@@ -130,7 +130,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));
@@ -151,7 +151,7 @@ public void shouldGetRelatedObservations() throws Exception {
 		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));
@@ -160,7 +160,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));
@@ -177,11 +177,11 @@ public void shouldGetRelatedObservations() throws Exception {
 		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);
 
@@ -271,6 +271,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 {
@@ -317,7 +326,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());
 	}
 
@@ -330,7 +339,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());
@@ -348,4 +357,52 @@ public void isObsGrouping_shouldIncludeVoidedObs() throws Exception {
 		parent.addGroupMember(child);
 		assertTrue("When checking for Obs grouping, should include voided Obs", parent.isObsGrouping());
 	}
+
+	/**
+	 * @see Obs#getValueAsString(Locale)
+	 * @verifies use commas or decimal places depending on locale
+	 */
+	@Test
+	public void getValueAsString_shouldUseCommasOrDecimalPlacesDependingOnLocale() throws Exception {
+		Obs obs = new Obs();
+		obs.setValueNumeric(123456789.3);
+		String str = "123456789,3";
+		Assert.assertEquals(str, obs.getValueAsString(Locale.GERMAN));
+	}
+
+	/**
+	 * @see Obs#getValueAsString(Locale)
+	 * @verifies not use thousand separator
+	 */
+	@Test
+	public void getValueAsString_shouldNotUseThousandSeparator() throws Exception {
+		Obs obs = new Obs();
+		obs.setValueNumeric(123456789.0);
+		String str = "123456789.0";
+		Assert.assertEquals(str, obs.getValueAsString(Locale.ENGLISH));
+	}
+
+	/**
+	 * @see Obs#getValueAsString(Locale)
+	 * @verifies return regular number for size of zero to or greater than ten digits
+	 */
+	@Test
+	public void getValueAsString_shouldReturnRegularNumberForSizeOfZeroToOrGreaterThanTenDigits() throws Exception {
+		Obs obs = new Obs();
+		obs.setValueNumeric(1234567890.0);
+		String str = "1234567890.0";
+		Assert.assertEquals(str, obs.getValueAsString(Locale.ENGLISH));
+	}
+
+	/**
+	 * @see Obs#getValueAsString(Locale)
+	 * @verifies return regular number if decimal places are as high as six
+	 */
+	@Test
+	public void getValueAsString_shouldReturnRegularNumberIfDecimalPlacesAreAsHighAsSix() throws Exception {
+		Obs obs = new Obs();
+		obs.setValueNumeric(123456789.012345);
+		String str = "123456789.012345";
+		Assert.assertEquals(str, obs.getValueAsString(Locale.ENGLISH));
+	}
 }