Re: [slf4j-dev] svn commit: r1086 - in slf4j/trunk/slf4j-api/src: main/java/org/slf4j/helpers test/java/org/slf4j/helpers
Jörn Huxhorn wrote: Are you serious? I was. I once heard a lecture on logging and the prof said that the main objective of any logging framework must be to not break the application it is logging. Absolutely. From this point of view - that I absolutely share - the quality of the slf4j framework would be lower after applying toString for arrays than before since now there is a real possibility to crash an application by simply issuing a log message. Agreed. It's also not our task to define that recursive arrays are a bug. With the previous implementation a log message containing such an array would have worked, with the new implementation it would crash the application. Agreed. Whether something like recursive arrays makes sense is purely application specific. Agreed. Beside that, the fix is really simple... OK, you've got me convinced. Consider the fix done. For the sake of this and future discussions, let me mention that one of SLF4J's main selling points is its simplicity. Compared to JCL, SLF4J way of binding to the underlying logging system is outrageously simple. It covers fewer use cases than JCL. But, as SLF4J is simpler it is also more robust and as you pointed out, robustness must be an essential characteristic of a logging framework. In short, simplicity of implementation matters. More to the point, the bug under consideration is not insidious. If a client has cyclical arrays and attempts to log them, their application will crash immediately with a clear pointer to MessageFormatter.deeplyAppendParameter. They will complain about the lack of support for cyclical arrays and we will quickly provide a fix. Here, I am assuming that non-cyclical arrays during development do not suddenly become cyclical in production. So instead of supporting cyclical arrays today, we can provide them tomorrow, but only when a user asks for it. This is just an application of the 1$ Today Is Worth More Than 1$ Tomorrow principle -- avoiding adding features that users don't seem to need today and hence saving a few cycles of my development time and more significantly a few brain cycles of people trying to read SLF4J code. However, as you observed, the fix is simple and elegant, so there is no point in procrastinating the implementation of a simple solution. -- Ceki Gülcü ___ dev mailing list dev@slf4j.org http://www.slf4j.org/mailman/listinfo/dev
Re: [slf4j-dev] svn commit: r1086 - in slf4j/trunk/slf4j-api/src: main/java/org/slf4j/helpers test/java/org/slf4j/helpers
Hi Ceki.
You forgot the double[] in your last commit.
Regards, Joern.
On Thu, Jul 31, 2008 at 10:51 PM, [EMAIL PROTECTED] wrote:
Author: ceki
Date: Thu Jul 31 22:51:04 2008
New Revision: 1086
Modified:
slf4j/trunk/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java
slf4j/trunk/slf4j-api/src/test/java/org/slf4j/helpers/MessageFormatterTest.java
Log:
- add support for array values in parameters
For example,
log.debug(a:{},i:{}, A, new int[] {1, 2}});
will print as a:A,i:[1, 2] instead of a:A,i:[EMAIL PROTECTED] as
previously
log.debug(a:{},b:{},i:{},, new Object[] {A, B, new int[] {1, 2}});
will print as a:A,b:B,i:[1, 2] instead of a:A,b:B,i:[EMAIL PROTECTED] as
previously
This enhancement was proposed by lizongbo
Modified:
slf4j/trunk/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java
==
---
slf4j/trunk/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java
(original)
+++
slf4j/trunk/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java
Thu Jul 31 22:51:04 2008
@@ -24,12 +24,20 @@
package org.slf4j.helpers;
+import java.util.Arrays;
+
+// contributors: lizongbo
+
/**
* Formats messages according to very simple substitution rules.
Substitutions
* can be made 1, 2 or more arguments.
* p
* For example,
- * preMessageFormatter.format(quot;Hi {}.quot;,
quot;therequot;);/pre
+ *
+ * pre
+ * MessageFormatter.format(quot;Hi {}.quot;, quot;therequot;);
+ * /pre
+ *
* will return the string Hi there..
* p
* The {} pair is called the emformatting anchor/em. It serves to
@@ -40,14 +48,22 @@
* pattern itself but do not want them to be interpreted as a formatting
* anchors, you can escape the '{' character with '\', that is the
backslash
* character. Only the '{' character should be escaped. There is no need to
- * escape the '}' character. For example,
- * preMessageFormatter.format(quot;Set \\{1,2,3} is not equal to
{}.quot;, quot;1,2quot;);/pre
- * will return the string Set {1,2,3} is not equal to 1,2..
+ * escape the '}' character. For example,
+ *
+ * pre
+ * MessageFormatter.format(quot;Set \\{1,2,3} is not equal to {}.quot;,
quot;1,2quot;);
+ * /pre
+ *
+ * will return the string Set {1,2,3} is not equal to 1,2..
*
* p
- * The escaping behavior just described can be overridden by
- * escaping the escape character '\'. Calling
- * preMessageFormatter.format(quot;File name is C:{}.quot;,
quot;file.zipquot;);/pre
+ * The escaping behavior just described can be overridden by escaping the
escape
+ * character '\'. Calling
+ *
+ * pre
+ * MessageFormatter.format(quot;File name is C:{}.quot;,
quot;file.zipquot;);
+ * /pre
+ *
* will return the string File name is C:\file.zip.
*
* p
@@ -60,7 +76,7 @@
static final char DELIM_START = '{';
static final char DELIM_STOP = '}';
private static final char ESCAPE_CHAR = '\\';
-
+
/**
* Performs single argument substitution for the 'messagePattern' passed
as
* parameter.
@@ -75,9 +91,10 @@
* p
*
* @param messagePattern
- * The message pattern which will be parsed and formatted
+ *The message pattern which will be parsed and formatted
* @param argument
- * The argument to be substituted in place of the formatting
anchor
+ *The argument to be substituted in place of the
formatting
+ *anchor
* @return The formatted message
*/
public static String format(String messagePattern, Object arg) {
@@ -98,13 +115,13 @@
* will return the string Hi Alice. My name is Bob..
*
* @param messagePattern
- * The message pattern which will be parsed and formatted
+ *The message pattern which will be parsed and formatted
* @param arg1
- * The argument to be substituted in place of the first
formatting
- * anchor
+ *The argument to be substituted in place of the first
+ *formatting anchor
* @param arg2
- * The argument to be substituted in place of the second
formatting
- * anchor
+ *The argument to be substituted in place of the second
+ *formatting anchor
* @return The formatted message
*/
public static String format(String messagePattern, Object arg1, Object
arg2) {
@@ -117,10 +134,10 @@
* arguments can be passed in an array.
*
* @param messagePattern
- * The message pattern which will be parsed and formatted
+ *The message pattern which will be parsed and formatted
* @param argArray
- * An array of arguments to be substituted in place of
formatting
- * anchors
+ *An array of arguments to be substituted in place of
+ *
Re: [slf4j-dev] svn commit: r1086 - in slf4j/trunk/slf4j-api/src: main/java/org/slf4j/helpers test/java/org/slf4j/helpers
Hi Jörn,
At least someone is watching over the code. Cool.
I just realized that toString(primitive[]) methods in Arrays class were
introduced in JDK 1.5. SLF4J needs to work on JDK 1.4 if not 1.3. Yikes.
Jörn Huxhorn wrote:
Hi Ceki.
You forgot the double[] in your last commit.
Regards, Joern.
On Thu, Jul 31, 2008 at 10:51 PM, [EMAIL PROTECTED] wrote:
Author: ceki
Date: Thu Jul 31 22:51:04 2008
New Revision: 1086
Modified:
slf4j/trunk/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java
slf4j/trunk/slf4j-api/src/test/java/org/slf4j/helpers/MessageFormatterTest.java
Log:
- add support for array values in parameters
For example,
log.debug(a:{},i:{}, A, new int[] {1, 2}});
will print as a:A,i:[1, 2] instead of a:A,i:[EMAIL PROTECTED] as
previously
log.debug(a:{},b:{},i:{},, new Object[] {A, B, new int[] {1, 2}});
will print as a:A,b:B,i:[1, 2] instead of a:A,b:B,i:[EMAIL PROTECTED] as
previously
This enhancement was proposed by lizongbo
Modified:
slf4j/trunk/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java
==
---
slf4j/trunk/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java
(original)
+++
slf4j/trunk/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java
Thu Jul 31 22:51:04 2008
@@ -24,12 +24,20 @@
package org.slf4j.helpers;
+import java.util.Arrays;
+
+// contributors: lizongbo
+
/**
* Formats messages according to very simple substitution rules.
Substitutions
* can be made 1, 2 or more arguments.
* p
* For example,
- * preMessageFormatter.format(quot;Hi {}.quot;,
quot;therequot;);/pre
+ *
+ * pre
+ * MessageFormatter.format(quot;Hi {}.quot;, quot;therequot;);
+ * /pre
+ *
* will return the string Hi there..
* p
* The {} pair is called the emformatting anchor/em. It serves to
@@ -40,14 +48,22 @@
* pattern itself but do not want them to be interpreted as a formatting
* anchors, you can escape the '{' character with '\', that is the
backslash
* character. Only the '{' character should be escaped. There is no need to
- * escape the '}' character. For example,
- * preMessageFormatter.format(quot;Set \\{1,2,3} is not equal to
{}.quot;, quot;1,2quot;);/pre
- * will return the string Set {1,2,3} is not equal to 1,2..
+ * escape the '}' character. For example,
+ *
+ * pre
+ * MessageFormatter.format(quot;Set \\{1,2,3} is not equal to {}.quot;,
quot;1,2quot;);
+ * /pre
+ *
+ * will return the string Set {1,2,3} is not equal to 1,2..
*
* p
- * The escaping behavior just described can be overridden by
- * escaping the escape character '\'. Calling
- * preMessageFormatter.format(quot;File name is C:{}.quot;,
quot;file.zipquot;);/pre
+ * The escaping behavior just described can be overridden by escaping the
escape
+ * character '\'. Calling
+ *
+ * pre
+ * MessageFormatter.format(quot;File name is C:{}.quot;,
quot;file.zipquot;);
+ * /pre
+ *
* will return the string File name is C:\file.zip.
*
* p
@@ -60,7 +76,7 @@
static final char DELIM_START = '{';
static final char DELIM_STOP = '}';
private static final char ESCAPE_CHAR = '\\';
-
+
/**
* Performs single argument substitution for the 'messagePattern' passed
as
* parameter.
@@ -75,9 +91,10 @@
* p
*
* @param messagePattern
- * The message pattern which will be parsed and formatted
+ *The message pattern which will be parsed and formatted
* @param argument
- * The argument to be substituted in place of the formatting
anchor
+ *The argument to be substituted in place of the
formatting
+ *anchor
* @return The formatted message
*/
public static String format(String messagePattern, Object arg) {
@@ -98,13 +115,13 @@
* will return the string Hi Alice. My name is Bob..
*
* @param messagePattern
- * The message pattern which will be parsed and formatted
+ *The message pattern which will be parsed and formatted
* @param arg1
- * The argument to be substituted in place of the first
formatting
- * anchor
+ *The argument to be substituted in place of the first
+ *formatting anchor
* @param arg2
- * The argument to be substituted in place of the second
formatting
- * anchor
+ *The argument to be substituted in place of the second
+ *formatting anchor
* @return The formatted message
*/
public static String format(String messagePattern, Object arg1, Object
arg2) {
@@ -117,10 +134,10 @@
* arguments can be passed in an array.
*
* @param messagePattern
- * The message pattern which will be parsed and formatted
+ *The message pattern which will be parsed and
Re: [slf4j-dev] svn commit: r1086 - in slf4j/trunk/slf4j-api/src: main/java/org/slf4j/helpers test/java/org/slf4j/helpers
Jörn Huxhorn wrote: And I just found out that it would be more appropriate to use Arrays.deepToString((Object[]) o); in case of Object[]. Right, except that deepToString is JDK 1.5. Double yikes. -- Ceki Gülcü QOS.ch is looking to hire talented developers located in Switzerland to work on cutting-edge software projects. If you think you are qualified, then please contact [EMAIL PROTECTED] ___ dev mailing list dev@slf4j.org http://www.slf4j.org/mailman/listinfo/dev
Re: [slf4j-dev] svn commit: r1086 - in slf4j/trunk/slf4j-api/src: main/java/org/slf4j/helpers test/java/org/slf4j/helpers
Damn, you are right! I'm not watching out for 1.5 since I have a =1.5 requirement for Lilith. There's still a problem with your deeplyAppendParameter method: It's not handling recursive arrays yet. While this isn't a terribly realistic case it would cause a stack overflow at the moment. Take a look at the 1.5 implementation of deepToString, it's using a HashSet (called dejaVu :D) to prevent this. Yes, I'm watching your code... I hope that doesn't make you paranoid ;) Joern. On Fri, Aug 1, 2008 at 4:18 PM, Ceki Gulcu [EMAIL PROTECTED] wrote: Jörn Huxhorn wrote: And I just found out that it would be more appropriate to use Arrays.deepToString((Object[]) o); in case of Object[]. Right, except that deepToString is JDK 1.5. Double yikes. -- Ceki Gülcü QOS.ch is looking to hire talented developers located in Switzerland to work on cutting-edge software projects. If you think you are qualified, then please contact [EMAIL PROTECTED] ___ dev mailing list dev@slf4j.org http://www.slf4j.org/mailman/listinfo/dev ___ dev mailing list dev@slf4j.org http://www.slf4j.org/mailman/listinfo/dev
Re: [slf4j-dev] svn commit: r1086 - in slf4j/trunk/slf4j-api/src: main/java/org/slf4j/helpers test/java/org/slf4j/helpers
Jörn Huxhorn wrote: Damn, you are right! There's still a problem with your deeplyAppendParameter method: It's not handling recursive arrays yet. Yes, indeed. While this isn't a terribly realistic case it would cause a stack overflow at the moment. Take a look at the 1.5 implementation of deepToString, it's using a HashSet (called dejaVu :D) to prevent this. The probability of passing a cyclical array is very low. Moreover, if and when it happens, the user will immediately know the cause of the failure. The StackOverflowError will point to MessageFormatter.deeplyAppendParameter method, so it will be easy to identify. If the bug were hard to identify, then it would have been an entirely different matter. At this stage, I am just too lazy to be bothered. Yes, I'm watching your code... I hope that doesn't make you paranoid ;) Not at all. On the contrary, it keeps me sharp. Well, only relatively sharp. You can lead a camel to water but you can't make it drink. :-) -- Ceki Gülcü QOS.ch is looking to hire talented developers located in Switzerland to work on cutting-edge software projects. If you think you are qualified, then please contact [EMAIL PROTECTED] ___ dev mailing list dev@slf4j.org http://www.slf4j.org/mailman/listinfo/dev
