Hi Martin Grigorov,
But i made naming proposals:
AbstractAttributeRegister
| |
AttributeAppender AttributePrepender
My pain in the ... is only that we have now 2 more constructors. This
(in my opinion)
really important class should be clear in design and responsibility.
Another pain could become important if we decide to exchange the default
values
for prepend. Wicket did that already in history and it lead to confusion
and required
app changes.
The sense in "the one common parent" would be to provide the abstract method
for value registering. The subclasses would have to implement that.
package org.apache.wicket.behavior;
import org.apache.wicket.AttributeModifier;
import org.apache.wicket.model.IModel;
import org.apache.wicket.util.string.AppendingStringBuffer;
import org.apache.wicket.util.string.Strings;
public class AbstractAttributeRegister extends AttributeModifier
{
private static final long serialVersionUID = 1L;
private final String separator;
public AbstractAttributeRegister(String attribute, boolean
addAttributeIfNotPresent,
IModel<?> appendModel, String separator)
{
this(attribute, addAttributeIfNotPresent, appendModel, separator);
}
public AbstractAttributeRegister(String attribute, IModel<?>
appendModel, String separator)
{
this(attribute, true, appendModel, separator);
}
/**
* @see
org.apache.wicket.AttributeModifier#newValue(java.lang.String,
java.lang.String)
*/
@Override
protected String newValue(String currentValue, String appendValue)
{
// Shortcut for empty values
if (Strings.isEmpty(currentValue))
{
return appendValue != null ? appendValue : "";
}
else if (Strings.isEmpty(appendValue))
{
return currentValue;
}
final AppendingStringBuffer sb = new
AppendingStringBuffer(currentValue.length() +
appendValue.length() + separator.length());
return registerValue(sb, currentValue, appendValue);
}
protected abstract String registerValue(AppendingStringBuffer
target, String currentValue, String appendValue);
}
public class AttributeAppender extends AbstractAttributeRegister
{
private static final long serialVersionUID = 1L;
public AttributeAppender(String attribute, boolean
addAttributeIfNotPresent,
IModel<?> appendModel, String separator)
{
super(attribute, addAttributeIfNotPresent, appendModel, separator);
}
public AttributeAppender(String attribute, IModel<?> appendModel,
String separator)
{
super(attribute, appendModel, separator);
}
@Override
protected String registerValue(AppendingStringBuffer target, String
currentValue, String value, String separator)
{
sb.append(currentValue);
sb.append(separator);
sb.append(value);
return sb.toString();
}
}
public class AttributePrepender extends AbstractAttributeRegister
{
private static final long serialVersionUID = 1L;
public AttributePrepender(String attribute, boolean
addAttributeIfNotPresent,
IModel<?> appendModel, String separator)
{
super(attribute, addAttributeIfNotPresent, appendModel, separator);
}
public AttributePrepender(String attribute, IModel<?> appendModel,
String separator)
{
super(attribute, appendModel, separator);
}
@Override
protected String registerValue(AppendingStringBuffer target, String
currentValue, String value, String separator)
{
sb.append(value);
sb.append(separator);
sb.append(currentValue);
return sb.toString();
}
}
Hi,
I also had the same thoughts when I added the flag (as the patch
suggested) but AttributeAppender is a class with just constructor
overrides and one method override (#newValue()). If we introduce yet
another class for prepend then there is no sense in "the one common
parent" because they have nothing to share. Both of them will have the
same number of constructors and this override of #newValue().
I agree that now the name AttributeAppender is not the best but I
cannot find a better one and even if we find it then we will have to
rename this class which I think is used a lot
On Wed, Jun 8, 2011 at 8:53 AM, Per Newgro<[email protected]> wrote:
Hi,
i've traced changes in 1.5-RC4.2. Found the issue 3552 [
https://issues.apache.org/jira/browse/WICKET-3552 ]. If i'm not
completely in the wood the solution breaks the
SingleResponsibilityPrinciple by adding a flag to the constructor
which negates the class behavior.
Wouldn't it be more clear and reusable if we had another
AttributePrepender which acts like the AttributeAppender.
Both could extend the base-class AbstractAttributeRegister or
something. Then only the different part of the newValue method has
to be implemented by the concrete class.
I only ask because if 1.5 is out it will be hard to exchange because
many apps will be upgraded and feature is maybe used :-).
What do you think?
Cheers
Per
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
