Jira and up and running and the last time I checked it still supports attaching files (patches) :-)
If you have the karma you can miss the attaching part and commit directly in SVN dashorst ^^ 2011/6/8 Robert Dahlström <[email protected]>: > +1 on skipping a boolean flag in the constructor. Igors suggestion to > provide a mode parameter feels like a good one. > > /Robert > > On 06/08/2011 10:18 AM, Martin Grigorov wrote: >> >> Hi Per, >> >> I see your point. >> It is just my opinion (and ticket's reporter) that this additional >> boolean parameter to the constructor is adds much less noise than >> adding additional 2 classes. >> If the name of AttributeAppender is changed to AtrributeAdder (because >> we are used that add(obj) appends and add(0, obj) prepends) then it >> will be much cleaner. >> >> Igor's suggestion also sounds OK to me. Then we can either deprecate >> AttributeAppender or add AttributePrepender and both of them will just >> have constructors that pass the proper mode to AttributeModifier. >> >> Create a ticket. >> >> On Wed, Jun 8, 2011 at 10:04 AM, Per Newgro<[email protected]> wrote: >>> >>> 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] >>> >>> >> >> >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > > -- Martin Grigorov jWeekend Training, Consulting, Development http://jWeekend.com --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
