You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fortress@directory.apache.org by Yudhi Karunia Surtan <yu...@apache.org> on 2018/04/04 07:26:22 UTC
Re: Process of making changes at trunk
Guys,
Finally make my first commit to apache git repository.
Thanks for guide me until here.
I also find there are some duplication for some code like this :
Properties properties = null;
List<Props.Entry> props = inProps.getEntry();
if ( props.size() > 0 )
{
properties = new Properties();
//int size = props.size();
for ( Props.Entry entry : props )
{
String key = entry.getKey();
String val = entry.getValue();
properties.setProperty( key, val );
}
}
return properties;
at org.apache.directory.fortress.core.model.Group,
org.apache.directory.fortress.core.model.Permission,
org.apache.directory.fortress.core.model.PermObj,
org.apache.directory.fortress.core.model.Role,
org.apache.directory.fortress.core.model.User.
Currently we have RestUtils.getProperties What do you think if above
classes using RestUtils method to remove duplication? However, if we do
that from the dependency point of view, i think it is a bit awkward because
"model" package depends on "rest" package.
On Tue, Feb 13, 2018 at 10:09 PM, Shawn McKinney <sm...@apache.org>
wrote:
>
> > On Feb 13, 2018, at 9:06 AM, Shawn McKinney <sm...@apache.org>
> wrote:
> >
> > I always create an issue in JIRA, with a detailed description of the
> problem and the fix, and use the jira issue #, along with a short desc, in
> the commit comments.
> >
> > e.g. FC-231 - DAO's should be package private
> >
> > That way we can tie the change back to the issue, where longer
> description of the change may take place.
>
> Another advantage of using JIRA to manage issues, we have a nice list of
> changes, for the release notes.
Re: Process of making changes at trunk
Posted by Shawn McKinney <sm...@apache.org>.
> On Apr 4, 2018, at 2:26 AM, Yudhi Karunia Surtan <yu...@apache.org> wrote:
>
> Currently we have RestUtils.getProperties What do you think if above
> classes using RestUtils method to remove duplication? However, if we do
> that from the dependency point of view, i think it is a bit awkward because
> "model" package depends on "rest" package.
Agreed, how about we create a sep utils in the model package and move the code into there?
Re: Process of making changes at trunk
Posted by Shawn McKinney <sm...@apache.org>.
> On Apr 4, 2018, at 11:47 PM, Emmanuel Lécharny <el...@gmail.com> wrote:
>
> Le 05/04/2018 à 00:57, Shawn McKinney a écrit :
>>
>>> On Apr 4, 2018, at 4:18 AM, Emmanuel Lécharny <el...@gmail.com> wrote:
>>>
>>> One more thing :
>>>
>>> if ( props.size() > 0 )
>>>
>>> should be
>>>
>>> if ( !props.isEmpty() )
>>
>> That won’t work, props is not java.util.Properties, it’s:
>
> List<Props.Entry> props = inProps.getEntry();
>
> So it *must* implement List.isEmpty()...
I stand corrected. Thanks
Re: Process of making changes at trunk
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 05/04/2018 à 00:57, Shawn McKinney a écrit :
>
>> On Apr 4, 2018, at 4:18 AM, Emmanuel Lécharny <el...@gmail.com> wrote:
>>
>> One more thing :
>>
>> if ( props.size() > 0 )
>>
>> should be
>>
>> if ( !props.isEmpty() )
>
> That won’t work, props is not java.util.Properties, it’s:
List<Props.Entry> props = inProps.getEntry();
So it *must* implement List.isEmpty()...
--
Emmanuel Lecharny
Symas.com
directory.apache.org
Re: Process of making changes at trunk
Posted by Shawn McKinney <sm...@apache.org>.
> On Apr 4, 2018, at 4:18 AM, Emmanuel Lécharny <el...@gmail.com> wrote:
>
> One more thing :
>
> if ( props.size() > 0 )
>
> should be
>
> if ( !props.isEmpty() )
That won’t work, props is not java.util.Properties, it’s:
public class Props extends FortEntity implements Serializable
{
/** Default serialVersionUID */
private static final long serialVersionUID = 1L;
private List<Props.Entry> entry;
Sort of a container class to carry properties as a list of name/values when it’s time to serialize via jaxb. The rationale for converting escapes me atm, other than it (serialization) didn’t work with attribute type of java.util.Properties.
Might be worth a revisit, to clean this up, i.e. use the java.util version and not this other one. We’d be able to also remove the redundant code Yudhi mentioned, i.e. using the delete key.
Shawn
Re: Process of making changes at trunk
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 04/04/2018 à 09:26, Yudhi Karunia Surtan a écrit :
> Guys,
>
> Finally make my first commit to apache git repository.
> Thanks for guide me until here.
>
>
> I also find there are some duplication for some code like this :
>
> Properties properties = null;
> List<Props.Entry> props = inProps.getEntry();
> if ( props.size() > 0 )
> {
> properties = new Properties();
> //int size = props.size();
> for ( Props.Entry entry : props )
> {
> String key = entry.getKey();
> String val = entry.getValue();
> properties.setProperty( key, val );
> }
> }
> return properties;
>
>
> at org.apache.directory.fortress.core.model.Group,
> org.apache.directory.fortress.core.model.Permission,
> org.apache.directory.fortress.core.model.PermObj,
> org.apache.directory.fortress.core.model.Role,
> org.apache.directory.fortress.core.model.User.
>
> Currently we have RestUtils.getProperties What do you think if above
> classes using RestUtils method to remove duplication? However, if we do
> that from the dependency point of view, i think it is a bit awkward because
> "model" package depends on "rest" package.
I do think an utility function should be added to fortress-core, as each
project depends on it. It makes no sense to move that to the
fortress-rest module, IMHO.
One more thing :
if ( props.size() > 0 )
should be
if ( !props.isEmpty() )
And, yes, +1 to remove code duplication :-)
--
Emmanuel Lecharny
Symas.com
directory.apache.org