Congratulations on a useful project; here's my suggestion
Hi, First of all, I'm new to this project and I'd like to congratulate you on putting it together with your typical no-nonsense approach to APIs. Particular praise is worthy for the IMessageConveyor interface hiding the ugliness of ResourceBundle and MessageFormat within a single method with varargs - inspired simplicity! Secondly, I would like to suggest a use-case that your API does not (yet?) cover. Maybe this is an idea you could consider incorporating. Within an enterprise application, especially a web-based application for example, resources frequently need to be looked up and corresponding strings returned to the user's display. But I have found that there are times in such applications where I would prefer to defer the choice of locale. For example, I may have some 'library' code not concerned with the UI yet which produces messages that are fed back to the user. These are status messages, error messages etc. The classic way to deal with these is to return separate transfer objects and later construct messages from them, but this can make the parameterisation awkward and adds complexity. Maybe it would be simpler to return the message directly. The problem with the Cal10n API as it stands is that I need to know the locale before I can construct a MessageConveyor. Only then can I call the getMessage(E e, Object... args) method to get the string, including its formatted parameters. The problem with web applications is that only the UI tier knows the locale. It is incorrect for other tiers to assume a different locale and unwieldy to pass the correct locale through as a parameter. So it would typically be wrong for the lower tiers to use MessageConveyor. This is a shame and my suggestion is simply that an additional data transfer object is required as part of the Cal10n API to fill the gap. This is essentially a Message class containing the enumeration and the args but without the resource bundle/message format lookup having yet been performed. At a later stage, a getLocalizedMessage(Locale locale) method will yield the required String for a given locale. A desirable characteristic of Message is that it be immutable because this would make it safe to hold within Exceptions or to be shared between threads. In other words, it would be better for it not to be a classic mutable JavaBean data transfer object. Therefore it is arguable that a general interface is not needed and a concrete and final Message class would be completely sufficient. So here is a possible implementation for Message: public final class Message { private final Enum<?> e; private final Object[] args; public Message(Enum<?> e, Object... args) { this.e = e; this.args = args; } public String getLocalizedMessage(Locale locale) { final IMessageConveyor mc = new MessageConveyor(locale); return mc.getMessage(e, args); } @Override public String toString() { final StringBuilder b = new StringBuilder("Message("); b.append(e.name()); b.append(", ["); if (args != null) { String comma = ""; for (Object o : args) { b.append(comma).append(o); comma = ", "; } } b.append("])"); return b.toString(); } } In use, the 'library' (non-UI) code will have its own resource bundle and corresponding enum. It will create instances of Message to return to the UI tier, where the message string for the appropriate locale can be looked up and then presented to the user. It would be beneficial to override hashcode() and equals() methods appropriately but I haven't included that here. There may also be some performance value in holding transient caches of the toString() result and the hashcode, although I omitted this also. I wish the project success and I look forward to recommending it to my clients when it reaches a sufficient level of maturity. Regards, Rick Beton -- Big Bee Consultants Limited : Registered in England & Wales No. 6397941 Registered Office: 71 The Hundred, Romsey, Hampshire, SO51 8BZ
Hello Rick I have this use-case experience. I think your suggestion has two problems. Message class you suggested isn't immutable. Object is call by reference. And Message class must implement Serializable because Exception is serialized in RMI. However Object isn't ensure to implement Serializable. I think message need to be build as soon as possible to reduce several problems. If we want to change Locale of MessageConveyor by request or user, we can do it to use dependency injection. It is no need to know Locale. Takeshi Kondo On 2009/08/29, at 8:22, Rick Beton wrote:
Hi,
First of all, I'm new to this project and I'd like to congratulate you on putting it together with your typical no-nonsense approach to APIs. Particular praise is worthy for the IMessageConveyor interface hiding the ugliness of ResourceBundle and MessageFormat within a single method with varargs - inspired simplicity!
Secondly, I would like to suggest a use-case that your API does not (yet?) cover. Maybe this is an idea you could consider incorporating.
Within an enterprise application, especially a web-based application for example, resources frequently need to be looked up and corresponding strings returned to the user's display. But I have found that there are times in such applications where I would prefer to defer the choice of locale. For example, I may have some 'library' code not concerned with the UI yet which produces messages that are fed back to the user. These are status messages, error messages etc. The classic way to deal with these is to return separate transfer objects and later construct messages from them, but this can make the parameterisation awkward and adds complexity. Maybe it would be simpler to return the message directly.
The problem with the Cal10n API as it stands is that I need to know the locale before I can construct a MessageConveyor. Only then can I call the getMessage(E e, Object... args) method to get the string, including its formatted parameters. The problem with web applications is that only the UI tier knows the locale. It is incorrect for other tiers to assume a different locale and unwieldy to pass the correct locale through as a parameter. So it would typically be wrong for the lower tiers to use MessageConveyor.
This is a shame and my suggestion is simply that an additional data transfer object is required as part of the Cal10n API to fill the gap. This is essentially a Message class containing the enumeration and the args but without the resource bundle/message format lookup having yet been performed. At a later stage, a getLocalizedMessage(Locale locale) method will yield the required String for a given locale.
A desirable characteristic of Message is that it be immutable because this would make it safe to hold within Exceptions or to be shared between threads. In other words, it would be better for it not to be a classic mutable JavaBean data transfer object. Therefore it is arguable that a general interface is not needed and a concrete and final Message class would be completely sufficient. So here is a possible implementation for Message:
public final class Message { private final Enum<?> e; private final Object[] args;
public Message(Enum<?> e, Object... args) { this.e = e; this.args = args; }
public String getLocalizedMessage(Locale locale) { final IMessageConveyor mc = new MessageConveyor(locale); return mc.getMessage(e, args); }
@Override public String toString() { final StringBuilder b = new StringBuilder("Message("); b.append(e.name()); b.append(", ["); if (args != null) { String comma = ""; for (Object o : args) { b.append(comma).append(o); comma = ", "; } } b.append("])"); return b.toString(); } }
In use, the 'library' (non-UI) code will have its own resource bundle and corresponding enum. It will create instances of Message to return to the UI tier, where the message string for the appropriate locale can be looked up and then presented to the user.
It would be beneficial to override hashcode() and equals() methods appropriately but I haven't included that here. There may also be some performance value in holding transient caches of the toString() result and the hashcode, although I omitted this also.
I wish the project success and I look forward to recommending it to my clients when it reaches a sufficient level of maturity.
Regards, Rick Beton -- Big Bee Consultants Limited : Registered in England & Wales No. 6397941 Registered Office: 71 The Hundred, Romsey, Hampshire, SO51 8BZ
_______________________________________________ cal10n-dev mailing list cal10n-dev@qos.ch http://qos.ch/mailman/listinfo/cal10n-dev
I have developed the idea a bit further. I think ResourceMessage is a better name (Message could mean too many other things). This is my contribution for you to include in the API if you want to. Regards, Rick package ch.qos.cal10n; import java.io.Serializable; import java.util.Locale; /** * Holds a resource message with the locale lookup deferred until it is needed * or until the locale is known. Instances are immutable so are safe to use as * Exception data or share between threads. Note that the immutability is * actually dependent on the supplied args themselves being immutable. If * ResourceMessage instances are created that contain parameters that are * mutable, the results could be unpredictable. * * @author Rick Beton */ public final class ResourceMessage implements Serializable { private static final long serialVersionUID = 6660897864328889682L; private final Enum<?> e; private final Object[] args; // cache the result of toString private transient String string = null; // cache the result of hashCode private transient int hashcode = 0; /** * Constructs an instance. * * @param e * the key for the corresponding resource. * @param args * any message parameters, as required. */ public ResourceMessage(Enum<?> e, Object... args) { this.e = e; if (args != null) { // take defensive copy this.args = new Object[args.length]; System.arraycopy(args, 0, this.args, 0, args.length); } else { this.args = null; } } /** * Gets the localized message from the resource bundle, using the specified * locale. * * @return the message for the user */ public String getLocalizedMessage(Locale locale) { final IMessageConveyor mc = new MessageConveyor(locale); return mc.getMessage(e, args); } @Override public String toString() { if (string == null) { final StringBuilder b = new StringBuilder("Message("); b.append(e.name()); b.append(", ["); if (args != null) { String comma = ""; for (Object o : args) { b.append(comma).append(o); comma = ", "; } } b.append("])"); // there may be a thread race here but the outcome is never // uncertain string = b.toString(); } return string; } @Override public boolean equals(Object obj) { if (this == obj) { return true; } if (obj.getClass() != ResourceMessage.class) { return false; } final ResourceMessage other = (ResourceMessage) obj; if (!this.e.equals(other.e)) { return false; } else if (this.args == null && other.args == null) { return true; } else if (this.args == null || other.args == null) { return false; } else if (this.args.length != other.args.length) { return false; } for (int i = 0; i < args.length; i++) { if (this.args[i] == null && other.args[i] == null) { // ok } else if (this.args[i] == null && other.args[i] != null) { return false; } else if (!this.args[i].equals(other.args[i])) { return false; } } return true; } @Override public int hashCode() { if (hashcode == 0) { int newHashCode = 0; newHashCode = e.hashCode(); if (args != null) { for (Object o : args) { if (o != null) { newHashCode ^= o.hashCode(); } } } // there may be a thread race here but the outcome is never // uncertain hashcode = newHashCode; } return hashcode; } }
Thank you Rick. FYI, you could replace the comparison of this.args and other.args (about 10 lines) with just Arrays.equals(this.args, other.args). Moreover, you could implement hashCode as follows, which correct and efficient at the same time. Having args contribute to the hashCode is a waste of time. @Override public int hashCode() { return e.hashCode(); } How about renaming ResourceMessage as MessageParameterObj. See also http://www.refactoring.com/catalog/introduceParameterObject.html Cheers, Rick Beton wrote:
I have developed the idea a bit further. I think ResourceMessage is a better name (Message could mean too many other things). This is my contribution for you to include in the API if you want to.
Regards, Rick
package ch.qos.cal10n;
import java.io.Serializable; import java.util.Locale;
/** * Holds a resource message with the locale lookup deferred until it is needed * or until the locale is known. Instances are immutable so are safe to use as * Exception data or share between threads. Note that the immutability is * actually dependent on the supplied args themselves being immutable. If * ResourceMessage instances are created that contain parameters that are * mutable, the results could be unpredictable. * * @author Rick Beton */ public final class ResourceMessage implements Serializable { private static final long serialVersionUID = 6660897864328889682L;
private final Enum<?> e; private final Object[] args; // cache the result of toString private transient String string = null; // cache the result of hashCode private transient int hashcode = 0;
/** * Constructs an instance. * * @param e * the key for the corresponding resource. * @param args * any message parameters, as required. */ public ResourceMessage(Enum<?> e, Object... args) { this.e = e; if (args != null) { // take defensive copy this.args = new Object[args.length]; System.arraycopy(args, 0, this.args, 0, args.length); } else { this.args = null; } }
/** * Gets the localized message from the resource bundle, using the specified * locale. * * @return the message for the user */ public String getLocalizedMessage(Locale locale) { final IMessageConveyor mc = new MessageConveyor(locale); return mc.getMessage(e, args); }
@Override public String toString() { if (string == null) { final StringBuilder b = new StringBuilder("Message("); b.append(e.name <http://e.name>()); b.append(", ["); if (args != null) { String comma = ""; for (Object o : args) { b.append(comma).append(o); comma = ", "; } } b.append("])"); // there may be a thread race here but the outcome is never // uncertain string = b.toString(); } return string; }
@Override public boolean equals(Object obj) { if (this == obj) { return true; } if (obj.getClass() != ResourceMessage.class) { return false; }
final ResourceMessage other = (ResourceMessage) obj; if (!this.e.equals(other.e)) { return false; } else if (this.args == null && other.args == null) { return true; } else if (this.args == null || other.args == null) { return false; } else if (this.args.length != other.args.length) { return false; }
for (int i = 0; i < args.length; i++) { if (this.args[i] == null && other.args[i] == null) { // ok } else if (this.args[i] == null && other.args[i] != null) { return false; } else if (!this.args[i].equals(other.args[i])) { return false; } } return true; }
@Override public int hashCode() { if (hashcode == 0) { int newHashCode = 0; newHashCode = e.hashCode(); if (args != null) { for (Object o : args) { if (o != null) { newHashCode ^= o.hashCode(); } } } // there may be a thread race here but the outcome is never // uncertain hashcode = newHashCode; } return hashcode; } }
-- Ceki Gülcü Logback: The reliable, generic, fast and flexible logging framework for Java. http://logback.qos.ch
2009/8/31 Ceki Gulcu <ceki@qos.ch>
Thank you Rick. FYI, you could replace the comparison of this.args and other.args (about 10 lines) with just Arrays.equals(this.args, other.args).
Indeed - much simpler.
Moreover, you could implement hashCode as follows, which correct and efficient at the same time. Having args contribute to the hashCode is a waste of time.
@Override public int hashCode() { return e.hashCode(); }
That's probably better (simpler often means better), but now hashCode() and equals() have different behaviour so there ought to be a remark in the JavaDoc to say so. It could, for example, mean that a hashtable of instances may be less efficient because more linear searching might happen.
How about renaming ResourceMessage as MessageParameterObj. See also
http://www.refactoring.com/catalog/introduceParameterObject.html
I see from the Git patch email you've already done this. :-D -- Big Bee Consultants Limited : Registered in England & Wales No. 6397941 Registered Office: 71 The Hundred, Romsey, Hampshire, SO51 8BZ
Rick Beton wrote:
Moreover, you could implement hashCode as follows, which correct and efficient at the same time. Having args contribute to the hashCode is a waste of time.
@Override public int hashCode() { return e.hashCode(); }
That's probably better (simpler often means better), but now hashCode() and equals() have different behaviour so there ought to be a remark in the JavaDoc to say so. It could, for example, mean that a hashtable of instances may be less efficient because more linear searching might happen.
No, they do not have different behavior. Both methods act according to their respective contracts. Keep in mind that in a typical hashtable the actual size of the table is much smaller than the hash space. Thus, it is *perfectly* OK for hash values to collide from time to time. Not only is the simpler hashCode implementation simpler, it will also be faster in computing its result. Since occasional hash collisions are OK, the overall results are likely to be better as well. As I said, having args contribute to the hashCode is a waste of the code reader/reviewer's time as well as CPU cycles. (If unconcinved, write a small test case comparing the performance of the two variants.)
How about renaming ResourceMessage as MessageParameterObj. See also
http://www.refactoring.com/catalog/introduceParameterObject.html
I see from the Git patch email you've already done this. :-D
Indeed, but I am open to suggestions. -- Ceki Gülcü Logback: The reliable, generic, fast and flexible logging framework for Java. http://logback.qos.ch
2009/9/1 Ceki Gulcu <ceki@qos.ch>
No, they do not have different behavior. Both methods act according to their respective contracts. Keep in mind that in a typical hashtable the actual size of the table is much smaller than the hash space. Thus, it is *perfectly* OK for hash values to collide from time to time.
I'm happy with that, but I was thinking about the particular case of several instances sharing a common enum value but differing in their parameters. They would all be in the same hashtable bucket so linear searching would be needed. If this is sufficiently unlikely, then there is no need to include the parameters in the hash value. However, a better hash value is produced by the more complex code I wrote 3 days ago.
Not only is the simpler hashCode implementation simpler, it will also be faster in computing its result. Since occasional hash collisions are OK, the overall results are likely to be better as well. As I said, having args contribute to the hashCode is a waste of the code reader/reviewer's time as well as CPU cycles. (If unconcinved, write a small test case comparing the performance of the two variants.)
The performance is not an issue - in the code I emailed, I had a lazy cache of the hashcode to avoid the cost of recomputing it (and likewise for toString()). As far as complexity is concerned, your simpler approach is much much better. I don't really have a strong view on this and I'll leave the choice to you. Rick :-) -- Big Bee Consultants Limited : Registered in England & Wales No. 6397941 Registered Office: 71 The Hundred, Romsey, Hampshire, SO51 8BZ
Rick Beton wrote:
2009/9/1 Ceki Gulcu <ceki@qos.ch <mailto:ceki@qos.ch>>
No, they do not have different behavior. Both methods act according to their respective contracts. Keep in mind that in a typical hashtable the actual size of the table is much smaller than the hash space. Thus, it is *perfectly* OK for hash values to collide from time to time.
I'm happy with that, but I was thinking about the particular case of several instances sharing a common enum value but differing in their parameters. They would all be in the same hashtable bucket so linear searching would be needed. If this is sufficiently unlikely, then there is no need to include the parameters in the hash value. However, a better hash value is produced by the more complex code I wrote 3 days ago.
Indeed, it would be possible to come up with MessageParamObj instances with common keys but differing in their args. In that case, your impl would be perform better. However, I don't think such cases are likely to occur and can be safely ignored.
Not only is the simpler hashCode implementation simpler, it will also be faster in computing its result. Since occasional hash collisions are OK, the overall results are likely to be better as well. As I said, having args contribute to the hashCode is a waste of the code reader/reviewer's time as well as CPU cycles. (If unconcinved, write a small test case comparing the performance of the two variants.)
The performance is not an issue - in the code I emailed, I had a lazy cache of the hashcode to avoid the cost of recomputing it (and likewise for toString()).
You still have to compute the hash code the first time. It is often the case that the hash code for an object is computed only once and repeatedly on different object instances having the same or different contents. In such a scenario, there would be many cache misses...
As far as complexity is concerned, your simpler approach is much much better.
Well, it's simpler. :-) I still don't see an obvious use-case where MessageParamObj instances would be inserted into a map, not to say that it could not happen.
I don't really have a strong view on this and I'll leave the choice to you.
OK. Thank you for being cool headed about this. I appreciate that.
Rick :-) -- Ceki Gülcü Logback: The reliable, generic, fast and flexible logging framework for Java. http://logback.qos.ch
Takeshi Kondo wrote:
Hello Rick
I have this use-case experience.
I think your suggestion has two problems. Message class you suggested isn't immutable. Object is call by reference.
Indeed, that's an important concern.
And Message class must implement Serializable because Exception is serialized in RMI. However Object isn't ensure to implement Serializable.
Very good point. We have a similar problem in logback. It can be addressed by serializing all basic Java types (Integer, Long, Doublem etc) "as is" and transforming other types to String before serialization.
I think message need to be build as soon as possible to reduce several problems.
Rick's argument about needing to defer translation to a later stage is quite a valid one. Ralph Goers made a similar argument. Two requests this early in the project must correspond to a real need, don't you think? -- Ceki Gülcü Logback: The reliable, generic, fast and flexible logging framework for Java. http://logback.qos.ch
And Message class must implement Serializable because Exception is serialized in RMI. However Object isn't ensure to implement Serializable.
Very good point. We have a similar problem in logback. It can be addressed by serializing all basic Java types (Integer, Long, Doublem etc) "as is" and transforming other types to String before serialization. Indeed. Certainly, this implementation solve this problem.
I think message need to be build as soon as possible to reduce several problems.
Rick's argument about needing to defer translation to a later stage is quite a valid one. Ralph Goers made a similar argument. Two requests this early in the project must correspond to a real need, don't you think? Yes , I agree. I think it is good idea to prepare special ParameterObject for cal10n.
But I don't get use-case of this feature. What concerns me is this comment.
The problem with the Cal10n API as it stands is that I need to know the locale before I can construct a MessageConveyor.
This scenario can be solved by dependency injection. Because locale is given form outside business logic, we don't need to know locale in business logic. I think way to get outside resource is dependency injection. Could you tell me use-case about needing to defer translation to a later stage for my reference? On 2009/09/01, at 4:29, Ceki Gulcu wrote:
Takeshi Kondo wrote:
Hello Rick I have this use-case experience. I think your suggestion has two problems. Message class you suggested isn't immutable. Object is call by reference.
Indeed, that's an important concern.
And Message class must implement Serializable because Exception is serialized in RMI. However Object isn't ensure to implement Serializable.
Very good point. We have a similar problem in logback. It can be addressed by serializing all basic Java types (Integer, Long, Doublem etc) "as is" and transforming other types to String before serialization.
I think message need to be build as soon as possible to reduce several problems.
Rick's argument about needing to defer translation to a later stage is quite a valid one. Ralph Goers made a similar argument. Two requests this early in the project must correspond to a real need, don't you think?
-- Ceki Gülcü Logback: The reliable, generic, fast and flexible logging framework for Java. http://logback.qos.ch _______________________________________________ cal10n-dev mailing list cal10n-dev@qos.ch http://qos.ch/mailman/listinfo/cal10n-dev
2009/9/1 Takeshi Kondo <takeshi.kondo@gmail.com>
Could you tell me use-case about needing to defer translation to a later stage for my reference?
This was discussed earlier in this thread. Rick -- Big Bee Consultants Limited : Registered in England & Wales No. 6397941 Registered Office: 71 The Hundred, Romsey, Hampshire, SO51 8BZ
participants (3)
-
Ceki Gulcu -
Rick Beton -
Takeshi Kondo