Re: [handly-dev] Proposed API change: IModule

[Vladimir Piskarev <pisv@xxxxx>]

The Vlad's arguments sound more than convincing to me.

 

So, a warning to all:

 

The method ISourceElement#getSourceFile() is deprecated and will be removed in Handly 0.3.

 

Clients should no longer generally expect that a source construct is located in a source file (but may expect it to be contained in a IModule -- new API in Handly 0.3)

 

If you still need the ISourceFile for whatever reason, use #getAncestor(ISourceFile.class) -- and generally expect null as a valid return value, or use new API #getModule() -- with a downcast.

 

Thanks Vlad for your kind help in getting this right!

 

Best regards,

Vladimir

 

 

On Mon, Mar 2, 2015 at 12:04 PM, Vladimir Piskarev <pisv@xxxxx> wrote:

 > Is ISourceConstruct.getSourceFile really necessary anymore?

Good question!

I decided to keep it mainly for binary compatibility (though the contract compatibility is apparently broken, existing model implementations will not be affected by it).

Also, this method can conveniently save a downcast, especially for models that have only "source" modules, for which the check for the null return value is not necessary. (And that's the only models Handly supported thus far).

However, I agree that leaving this method in place may muddle the waters a bit.

I think we can deprecate and remove it eventually if the need arises. Do you feel that removing it now would result in a much cleaner solution?

Well, does a SourceConstruct ever care that the Module it's located in is a source file or something else? From what I can see, it doesn't and I think it shouldn't. 

I know that sometimes it's nice to have shortcut methods that assume things (I do that too, often), but what is ok in an application might be less so in a framework. I like your decision to keep Handly simple and not make assumptions - I think this is a case where you should stay true to that even if it breaks compatibility. After all, one of the main purposes of 0.x versions is to reveal the API: if one gets locked already at v0.2, then it could just as well be named 1.0. :-)

regards,

Vlad