famix-c review

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

famix-c review

Tudor Girba-2
Hi,

I went quickly through the changes from the FAMIX-C package. The Multivalue code looks good.

Here are two remarks:

1. I am not convinced about having CompilationUnit and Header be subclasses of CFile (which is a subclass of File). But, to make an opinion I would first like to see a model. @Alex, could you provide a simple MSE model to play with?


2. I would want to change is the names of the following two properties:

FAMIXCFile>>includedInFiles
        <MSEProperty: #includedInFiles type: #FAMIXInclude opposite: #includesFiles> <multivalued> <derived>
        <MSEComment: 'includedInFiles relationships, i.e. files that includes this file.'>
        ^ includedInFiles

FAMIXCFile>>includesFiles
        <MSEProperty: #includesFiles type: #FAMIXInclude opposite: #includedInFiles> <multivalued> <derived>
        <MSEComment: 'includesFiles relationships, i.e. files included by this file.'>
        ^ includingFiles

The names suggest that the result will be a collection of File objects, but it is actually a collection of Includes.

It is a bit tricky to name these selectors, but we could use the same convention we used for invocations, and we could have:
- includedInFiles ==> incomingIncludeRelations
- includesFiles ==> outgoingIncludeRelations

What do you think?

Cheers,
Doru

--
www.tudorgirba.com
www.feenk.com

"To lead is not to demand things, it is to make them happen."




_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.list.inf.unibe.ch/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: famix-c review

Tudor Girba-2
And I just discovered another thing.

FAMIXInclude overrides #from and #to, and when doing so, they also change the type of the property. 

FAMIXInclude>>from
<MSEProperty: #from type: #FAMIXFile opposite: #includingFiles>
^ from

FAMIXAssociation>>from
<MSEProperty: #from type: #FAMIXNamedEntity> <derived>
^ self subclassResponsibility

The property is marked as <derived> in the superclass, and this means that the value should come from somewhere else. Essentially, #from and #to are just polymorphic properties, and are not meant to store the concrete value. 

I now changed the code to have:
- FAMIXInclude not depend on from/to properties, but instead on new source/target ones.
- introduced incomingIncludeRelations/outgoingIncludeRelations in FAMIXCFile.

In the process, I also found that other places completely messed up the from/to definitions:

FAMIXInvocation>>from
<MSEProperty: #sender type: #FAMIXBehaviouralEntity opposite: #outgoingInvocations>
<MSEComment: 'Behavioural entity making the call. from-side of the association'>
^ self sender

One clear point here is that we need more constraints and tools to guide newcomers. I will follow up on that.

Cheers,
Doru


On Jul 2, 2016, at 4:43 PM, Tudor Girba <[hidden email]> wrote:

Hi,

I went quickly through the changes from the FAMIX-C package. The Multivalue code looks good.

Here are two remarks:

1. I am not convinced about having CompilationUnit and Header be subclasses of CFile (which is a subclass of File). But, to make an opinion I would first like to see a model. @Alex, could you provide a simple MSE model to play with?


2. I would want to change is the names of the following two properties:

FAMIXCFile>>includedInFiles
<MSEProperty: #includedInFiles type: #FAMIXInclude opposite: #includesFiles> <multivalued> <derived>
<MSEComment: 'includedInFiles relationships, i.e. files that includes this file.'>
^ includedInFiles

FAMIXCFile>>includesFiles
<MSEProperty: #includesFiles type: #FAMIXInclude opposite: #includedInFiles> <multivalued> <derived>
<MSEComment: 'includesFiles relationships, i.e. files included by this file.'>
^ includingFiles

The names suggest that the result will be a collection of File objects, but it is actually a collection of Includes.

It is a bit tricky to name these selectors, but we could use the same convention we used for invocations, and we could have:
- includedInFiles ==> incomingIncludeRelations
- includesFiles ==> outgoingIncludeRelations

What do you think?

Cheers,
Doru

--
www.tudorgirba.com
www.feenk.com

"To lead is not to demand things, it is to make them happen."





--
www.tudorgirba.com
www.feenk.com

"Next time you see your life passing by, say 'hi' and get to know her."





_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.list.inf.unibe.ch/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: famix-c review

abergel
Looks okay to me. Tests are kept green!

Alexandre


> On Jul 3, 2016, at 7:54 AM, Tudor Girba <[hidden email]> wrote:
>
> And I just discovered another thing.
>
> FAMIXInclude overrides #from and #to, and when doing so, they also change the type of the property.
>
> FAMIXInclude>>from
> <MSEProperty: #from type: #FAMIXFile opposite: #includingFiles>
> ^ from
>
> FAMIXAssociation>>from
> <MSEProperty: #from type: #FAMIXNamedEntity> <derived>
> ^ self subclassResponsibility
>
> The property is marked as <derived> in the superclass, and this means that the value should come from somewhere else. Essentially, #from and #to are just polymorphic properties, and are not meant to store the concrete value.
>
> I now changed the code to have:
> - FAMIXInclude not depend on from/to properties, but instead on new source/target ones.
> - introduced incomingIncludeRelations/outgoingIncludeRelations in FAMIXCFile.
>
> In the process, I also found that other places completely messed up the from/to definitions:
>
> FAMIXInvocation>>from
> <MSEProperty: #sender type: #FAMIXBehaviouralEntity opposite: #outgoingInvocations>
> <MSEComment: 'Behavioural entity making the call. from-side of the association'>
> ^ self sender
>
> One clear point here is that we need more constraints and tools to guide newcomers. I will follow up on that.
>
> Cheers,
> Doru
>
>
>> On Jul 2, 2016, at 4:43 PM, Tudor Girba <[hidden email]> wrote:
>>
>> Hi,
>>
>> I went quickly through the changes from the FAMIX-C package. The Multivalue code looks good.
>>
>> Here are two remarks:
>>
>> 1. I am not convinced about having CompilationUnit and Header be subclasses of CFile (which is a subclass of File). But, to make an opinion I would first like to see a model. @Alex, could you provide a simple MSE model to play with?
>>
>>
>> 2. I would want to change is the names of the following two properties:
>>
>> FAMIXCFile>>includedInFiles
>> <MSEProperty: #includedInFiles type: #FAMIXInclude opposite: #includesFiles> <multivalued> <derived>
>> <MSEComment: 'includedInFiles relationships, i.e. files that includes this file.'>
>> ^ includedInFiles
>>
>> FAMIXCFile>>includesFiles
>> <MSEProperty: #includesFiles type: #FAMIXInclude opposite: #includedInFiles> <multivalued> <derived>
>> <MSEComment: 'includesFiles relationships, i.e. files included by this file.'>
>> ^ includingFiles
>>
>> The names suggest that the result will be a collection of File objects, but it is actually a collection of Includes.
>>
>> It is a bit tricky to name these selectors, but we could use the same convention we used for invocations, and we could have:
>> - includedInFiles ==> incomingIncludeRelations
>> - includesFiles ==> outgoingIncludeRelations
>>
>> What do you think?
>>
>> Cheers,
>> Doru
>>
>> --
>> www.tudorgirba.com
>> www.feenk.com
>>
>> "To lead is not to demand things, it is to make them happen."
>>
>>
>>
>>
>
> --
> www.tudorgirba.com
> www.feenk.com
>
> "Next time you see your life passing by, say 'hi' and get to know her."
>
>
>
>
> _______________________________________________
> Moose-dev mailing list
> [hidden email]
> https://www.list.inf.unibe.ch/listinfo/moose-dev

--
_,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:
Alexandre Bergel  http://www.bergel.eu
^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.



_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.list.inf.unibe.ch/listinfo/moose-dev