NumberFormatException when trying to convert parameters to BigDecimal with German Locale

The default parameter converters use the default locale of the JVM - that is they are not specified when the Format is created. What is the default locale that you are running under?

Also, the Format objects should not be held as instance variables as they are not thread-safe. Just noticed that writing my own DateParameterConverter yesterday to do relative dates. We should hold the patterns and create the format from the pattern.

If you look closely, in the 2nd test method, the numberConverter instance is created using the localized germanNumbers format. So I would expect it to be used. Apart from that, the default Locale of my system is de_DE.

As for the object itself, you are generally correct, of course. A long time ago I added the appropriate detector to FindBugs myself

In this case, however, I am pretty sure the test is only run single-threaded, so no harm done.

Sorry, misread your comment's first part, you were referring to the Locale, not the format. Nevertheless, the default is German anyway.

Oh, sorry, just looked at the code and Locale.ENGLISH is the default in ParameterConverters. You can can change that in the configuration by using/creating a new ParameterConverters and specifying the locale in the constructor. It does NOT use the system default locale.

configuration.useParameterConverters(new ParameterConverters(configuration.stepMonitor(),Locale.GERMANY,",",false);

Let us know if configuring the ParameterConverters based on a "de_DE" locale works.

I also see that thread safety is sort of addressed there as well - but seems to be thread-safety around the manipulation of the list of parameter converters, not the converters themselves.

Changed the 2nd test method like this:

	@Test
	public void jbehaveConverter() throws Exception {
		// http://jira.codehaus.org/browse/JBEHAVE-739
		ParameterConverters parameterConverters = new ParameterConverters(
				new NullStepMonitor(), Locale.GERMANY, ",", false);

//		ParameterConverters.NumberConverter numberConverter = new ParameterConverters.NumberConverter(
				germanNumbers);
//		parameterConverters.addConverters(numberConverter);

		Object v = parameterConverters.convert(localizedString, BigDecimal.class);
		assertEquals(EXPECTED, v);
	}

and it still fails with the same Exception. I tried with and without the two commented lines in the middle. Still the same error.

I do not think this is a problem of the Locale not getting picked up, but with the canonicalization of the String in org.jbehave.core.steps.ParameterConverters.NumberConverter.canonicalize(String) which seems to be producing the wrong "1.000.000.000.00" String (see that there is a lot of dots that will be mistaken for more than one decimal separators by the BigDecimal(String) constructor.

I'm not quite sure what the reason for the canonicalize method is, either BigDecimal is not Locale capable and the method tries to fix that or it tries to fix a problem with the String parsing in BigDecimal, git blame gives this commit

https://github.com/jbehave/jbehave-core/commit/13c03225d597c412060280e4912ead14c01605ba

the reason why this is not working is probably that replace doesn't process regexp and replaceAll should be used instead, but I have to do a few more tests to actually be able to fix it.

(if the regex is applied correctly, I think the method will drop a minus sign)

I suggest this implementation of canonicalize.
I don't have access to GitHub currently, so I cannot create a patch yet. I will do so, including the tests I have locally for it, when no objections come up:

/**
	 * Canonicalized a number represented as a String to a format suitable
	 * for the BigDecimal(String) constructor. Takes into account the
	 * settings of the currently configures NumberFormat.
	 * 
	 * @param value
	 *            a localized number string
	 * @return the same number, but as a String suitable for consumption by
	 *         BigDecimal
	 */
	String canonicalize(String value) {
		char decimalPointSeparator = '.'; // default
		char minusSign = '-'; // default
		String rxNotDigits = "[^0-9]";
		StringBuilder returnBuilder = new StringBuilder(value.length());

		// override defaults according to numberFormat's settings
		if (numberFormat instanceof DecimalFormat) {
			DecimalFormatSymbols decimalFormatSymbols = ((DecimalFormat) numberFormat)
					.getDecimalFormatSymbols();
			minusSign = decimalFormatSymbols.getMinusSign();
			decimalPointSeparator = decimalFormatSymbols
					.getDecimalSeparator();
		}

		value = value.trim();
		int decimalPointPosition = value.lastIndexOf(decimalPointSeparator);
		boolean isNegative = value.charAt(0) == minusSign;

		if (isNegative) {
			returnBuilder.append('-'); // fixed "-" for BigDecimal
										// constructor
		}

		if (decimalPointPosition != -1) {
			String sf = value.substring(0, decimalPointPosition)
					.replaceAll(rxNotDigits, "");
			String dp = value.substring(decimalPointPosition + 1)
					.replaceAll(rxNotDigits, "");

			returnBuilder.append(sf);
			returnBuilder.append('.'); // fixed "." for BigDecimal
										// constructor
			returnBuilder.append(dp);

		} else {
			returnBuilder.append(value.replaceAll(rxNotDigits, ""));
		}
		return returnBuilder.toString();
	}

Applied patch with thanks.