moosetechnology / Famix

An abstract representation of source code. Famix is generic and can describe applications in multiple programming languages.
MIT License
12 stars 22 forks source link

Rework #numberOfComments? #735

Closed jecisc closed 3 months ago

jecisc commented 3 months ago

I found the method #numberOfComments and there are two points I would like to rework on it.

The first one is that this methods has a cache even if it is really fast and a setter to bypass the compute block of the cache. I think this was done in order to be able to set the number of comments even if we do not have the comments in the model. But I think that no parser currently working with Moose uses this feature. I would vote to simplify this and remove the setter and cache.

The second thing is that this method is reimplemented on JavaClass, StClass, JavaMethod and StMethod. For the methods the change is useless and I'll remove those. But for the classes, the behavior change and it also counts the comments of all the children.

To me it is bad to have one entity behaving in a different way than the other. After discussing with Benoit I think we should always return the number of comments of the entity without considering the children in #numberOfComments.

Does this seems good? @ClotildeToullec @NicolasAnquetil @anneetien ?

jecisc commented 3 months ago

The second point would also solve a problem. If we have a child of a class that does not have TWithComment, the method #numberOfComments fails. It's the reason I ended up on this method in the first place.

NicolasAnquetil commented 3 months ago

I would say no to 1st point and yes to 2nd.

For the frsit point:

jecisc commented 3 months ago

For 1: The downside of having this possibility is that each time you'll ask such properties it will cache them and increase the memory used by the model.

jecisc commented 3 months ago

I don't have any strong opinion on what to do about it so it's fine for me to let it