Expression Block

[Carsten Ziegeler <cz...@apache.org>]

Hi,

while briefly looking at the new and interesting expression blocks, some
things we should change came to my mind.

- We should use a consistent package naming scheme - I know that we
haven't done so in the past, but we should strive for this in new
blocks. So for the api, the package names should be
o.a.c.{block}.something where something can of course contain sub
packages. I know that this stuff comes from the old template block, but
it contains new interfaces so we can change the package as well.

- The same goes for the implementation, it should start with
o.a.c.{block}.somethingelse and do not use the same packages as the api.

- I'm wondering if we need a separate ExpressionCompiler interface. What
about adding the compile() method to the Expression which then eiter
compiles the addresses expression or returns a compiled version?

Carsten

-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Expression Block

[Carsten Ziegeler <cz...@apache.org>]

Ralph Goers wrote:
> Why?  It can create some interesting problems if the package for the
> impl is different than the API as you sometimes want to use package
> scope in the API and you won't be able to.  Frankly, creating an
> interface named Something and an impl named SomethingImpl should
> suffice.  I can't speak for Eclipse, but with IntelliJ finding the class
> is dirt simple so that shouldn't ever be a consideration.
> 
Hmm, I'm not sure if I want to use package scope for public api :)
Now, my main reason for this is OSGi where I can simply hide classes
based on packages. Split packages, containing both api and impl - or
public and private classes - is a pain there.

And speaking of ides, I would rather only import the api library to be
sure to not use private classes. In the case of also importing the
implementation, the different package names give at least a hint.

Carsten

-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Expression Block

[Ralph Goers <Ra...@dslextreme.com>]

Carsten Ziegeler wrote:
> Hi,
>
> while briefly looking at the new and interesting expression blocks, some
> things we should change came to my mind.
>
> - We should use a consistent package naming scheme - I know that we
> haven't done so in the past, but we should strive for this in new
> blocks. So for the api, the package names should be
> o.a.c.{block}.something where something can of course contain sub
> packages. I know that this stuff comes from the old template block, but
> it contains new interfaces so we can change the package as well.
>
> - The same goes for the implementation, it should start with
> o.a.c.{block}.somethingelse and do not use the same packages as the api.
>   
Why?  It can create some interesting problems if the package for the 
impl is different than the API as you sometimes want to use package 
scope in the API and you won't be able to.  Frankly, creating an 
interface named Something and an impl named SomethingImpl should 
suffice.  I can't speak for Eclipse, but with IntelliJ finding the class 
is dirt simple so that shouldn't ever be a consideration.

Ralph

Re: Expression Block

[Ralph Goers <Ra...@dslextreme.com>]

Oh darn. It is too late at night I guess. This example doesn't 
illustrate the problem as it doesn't matter whether the packages match.

Ralph Goers wrote:
> As I pointed out though, that can create problems for the 
> implementor.  As an example, you might have in an abstract class:
>
> public void something(String parm1) {
>    something(verify(parm1), null, null);
> }
>
> public void something(String parm1, String parm2) {
>    something(parm1, parm2, null);
> }
>
> public void something(String parm1, String, parm2, String parm3) {
>    doSomething(verifyP1(parm1), verifyP2(parm2), verifyP3(parm3));
> }
>
> protected abstract void doSomething(String parm1, String parm2, String 
> parm3);
>
> This construct can't be implemented if the packages aren't the same.
>> Carsten
>>
>>
>>   

Re: Expression Block

[Ralph Goers <Ra...@dslextreme.com>]

Carsten Ziegeler wrote:
> Ralph Goers wrote:
>   
>> Carsten Ziegeler wrote:
>>     
>>> Yes, that's correct, but the api might use o.a.c.expression while the
>>> implementation should use a different package like o.a.c.e.impl (or
>>> something more appropriate). We should not have two modules providing
>>> classes in the same package.
>>>
>>>   
>>>       
>> Here I don't agree. The API should only contain interfaces, abstract
>> classes, classes that are fairly simple (for example, if they have
>> nothing but getters and setters) or classes that would always be common
>> to any possible implementation.  The impl should do all the real work.
>>
>>     
> That's my saying :) I only try to say that all impl classes should use
> different package names
> than the api classes.
>   
As I pointed out though, that can create problems for the implementor.  
As an example, you might have in an abstract class:

public void something(String parm1) {
    something(verify(parm1), null, null);
}

public void something(String parm1, String parm2) {
    something(parm1, parm2, null);
}

public void something(String parm1, String, parm2, String parm3) {
    doSomething(verifyP1(parm1), verifyP2(parm2), verifyP3(parm3));
}

protected abstract void doSomething(String parm1, String parm2, String 
parm3);

This construct can't be implemented if the packages aren't the same.
> Carsten
>
>
>   

Re: Expression Block

[Carsten Ziegeler <cz...@apache.org>]

Ralph Goers wrote:
> Carsten Ziegeler wrote:
>>
>> Yes, that's correct, but the api might use o.a.c.expression while the
>> implementation should use a different package like o.a.c.e.impl (or
>> something more appropriate). We should not have two modules providing
>> classes in the same package.
>>
>>   
> Here I don't agree. The API should only contain interfaces, abstract
> classes, classes that are fairly simple (for example, if they have
> nothing but getters and setters) or classes that would always be common
> to any possible implementation.  The impl should do all the real work.
> 
That's my saying :) I only try to say that all impl classes should use
different package names
than the api classes.

Carsten


-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Expression Block

[Ralph Goers <Ra...@dslextreme.com>]

Carsten Ziegeler wrote:
>
> Yes, that's correct, but the api might use o.a.c.expression while the
> implementation should use a different package like o.a.c.e.impl (or
> something more appropriate). We should not have two modules providing
> classes in the same package.
>
>   
Here I don't agree. The API should only contain interfaces, abstract 
classes, classes that are fairly simple (for example, if they have 
nothing but getters and setters) or classes that would always be common 
to any possible implementation.  The impl should do all the real work.

Ralph

Re: Expression Block

[Ralph Goers <Ra...@dslextreme.com>]

Grzegorz Kossakowski wrote:
>
> I see. As long as Ralph do not convince us why this might be wrong I'm 
> fine with your proposal :-)
I'll think about this in my sleep and see if something more rational 
comes to mind in the morning. Probably not though.

Ralph

Re: Expression Block

[Grzegorz Kossakowski <gk...@apache.org>]

Carsten Ziegeler pisze:
> :) Okay, I see your point and you're most propably right - although we
> might have blocks that don't have COB-INF etc. and I might not want to
> use all core modules. So we should be prepared to make them optional
> where possible, e.g. if I don't want to have javascript support for the
> expression language stuff etc.

We should have clean, separated dependencies independently from if we deal with core module or 
block, at all. ;-)
Nevertheless, I think we agree on main directions. :)

> Ah, ok, I see, I would go with "el" for the package name then :) In
> general you are right that its better to use all segments but I guess
> that there will be exceptions to this rule :) So, best would be to have
> module names which consist of just one single name.

Ok, thanks for clarifying.

-- 
Grzegorz Kossakowski
http://reflectingonthevicissitudes.wordpress.com/
*** My Internet Service Provider breaks my internet connection                ***
*** incessantly so I'll not be able to respond to e-mails                     ***
*** regularly and my work will be somehow irregular.                          ***
*** I'm already trying to switch ISP but it will take handful amount of time. ***

Re: Expression Block

[Carsten Ziegeler <cz...@apache.org>]

Grzegorz Kossakowski wrote:
> I still have to disagree. It's also important from user's perspective
> because block is something that has structure of block (contains folders
> src/main/resources/META-INF and src/main/resources/COB-INF) and can be
> used as block.
> Core module is just piece of code that user is going to use without any
> consideration. Usually, user is not going to choose which core modules
> to use but she is going to choose blocks usable for developed application.
:) Okay, I see your point and you're most propably right - although we
might have blocks that don't have COB-INF etc. and I might not want to
use all core modules. So we should be prepared to make them optional
where possible, e.g. if I don't want to have javascript support for the
expression language stuff etc.

> Full name is "cocoon-expression-language-*" so module's name consists of
> segments "expression" and "language".
Ah, ok, I see, I would go with "el" for the package name then :) In
general you are right that its better to use all segments but I guess
that there will be exceptions to this rule :) So, best would be to have
module names which consist of just one single name.

Carsten


-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Expression Block

[Grzegorz Kossakowski <gk...@apache.org>]

Carsten Ziegeler pisze:
> Grzegorz Kossakowski wrote:
>> I'm don't think that expression is a block, I would rather say it is a
>> core module.
> :) Yes, you're right (although the difference from a user perspective is
> very subtle).

:)
I still have to disagree. It's also important from user's perspective because block is something 
that has structure of block (contains folders src/main/resources/META-INF and 
src/main/resources/COB-INF) and can be used as block.
Core module is just piece of code that user is going to use without any consideration. Usually, user 
is not going to choose which core modules to use but she is going to choose blocks usable for 
developed application.

> Oh, yes, that's fine - my comment was not meant as critics but just as a
> possible improvement.

Ok, I'll try to take care of it in the future, when we know how expressions are used and what 
functionality module contains.

> Hmm, what do you mean with segments?

Full name is "cocoon-expression-language-*" so module's name consists of segments "expression" and 
"language".

>> I'm lost here, could you elaborate?
>> I thought that both should use o.a.c.expression.language.something
> Yes, that's correct, but the api might use o.a.c.expression while the
> implementation should use a different package like o.a.c.e.impl (or
> something more appropriate). We should not have two modules providing
> classes in the same package.
> We can't ensure this for "old stuff" but we should do this for new stuff.

I see. As long as Ralph do not convince us why this might be wrong I'm fine with your proposal :-)

-- 
Grzegorz Kossakowski
http://reflectingonthevicissitudes.wordpress.com/
*** My Internet Service Provider breaks my internet connection                ***
*** incessantly so I'll not be able to respond to e-mails                     ***
*** regularly and my work will be somehow irregular.                          ***
*** I'm already trying to switch ISP but it will take handful amount of time. ***

Re: Expression Block

[Carsten Ziegeler <cz...@apache.org>]

Grzegorz Kossakowski wrote:
> I'm don't think that expression is a block, I would rather say it is a
> core module.
:) Yes, you're right (although the difference from a user perspective is
very subtle).

> I agree we should correct packages in expression module but as you
> already pointed out the mess comes from Template block.
> What's more, there was a lot of things to clean up (e.g. helper classes
> and object model initialization, test cases) that I had to do first. I
> haven't found enough free time to restructure package names.
Oh, yes, that's fine - my comment was not meant as critics but just as a
possible improvement.

> 
> When it comes to proposed naming guideline I'm fine with it, only
> wondering about block/modules with names made up from more than segment.
> Should package look like:
> o.a.c.{segment1}.{segment2}.something ?
> 
Hmm, what do you mean with segments?

>> - The same goes for the implementation, it should start with
>> o.a.c.{block}.somethingelse and do not use the same packages as the api.
> 
> I'm lost here, could you elaborate?
> I thought that both should use o.a.c.expression.language.something
Yes, that's correct, but the api might use o.a.c.expression while the
implementation should use a different package like o.a.c.e.impl (or
something more appropriate). We should not have two modules providing
classes in the same package.
We can't ensure this for "old stuff" but we should do this for new stuff.


Carsten

-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Expression Block

[Grzegorz Kossakowski <gk...@apache.org>]

Carsten Ziegeler pisze:
> Hi,

Hi

> while briefly looking at the new and interesting expression blocks, some
> things we should change came to my mind.

I'm don't think that expression is a block, I would rather say it is a core module.

> - We should use a consistent package naming scheme - I know that we
> haven't done so in the past, but we should strive for this in new
> blocks. So for the api, the package names should be
> o.a.c.{block}.something where something can of course contain sub
> packages. I know that this stuff comes from the old template block, but
> it contains new interfaces so we can change the package as well.

I agree we should correct packages in expression module but as you already pointed out the mess 
comes from Template block.
What's more, there was a lot of things to clean up (e.g. helper classes and object model 
initialization, test cases) that I had to do first. I haven't found enough free time to restructure 
package names.

When it comes to proposed naming guideline I'm fine with it, only wondering about block/modules with 
names made up from more than segment. Should package look like:
o.a.c.{segment1}.{segment2}.something ?

> - The same goes for the implementation, it should start with
> o.a.c.{block}.somethingelse and do not use the same packages as the api.

I'm lost here, could you elaborate?
I thought that both should use o.a.c.expression.language.something

> - I'm wondering if we need a separate ExpressionCompiler interface. What
> about adding the compile() method to the Expression which then eiter
> compiles the addresses expression or returns a compiled version?

Joerg already questioned[1] class names. Certainly we will have to take a look at this but my 
priority is COCOON-2110 issues for now.

[1] http://article.gmane.org/gmane.text.xml.cocoon.devel/74116
[2] https://issues.apache.org/jira/browse/COCOON-2110

-- 
Grzegorz Kossakowski
http://reflectingonthevicissitudes.wordpress.com/
*** My Internet Service Provider breaks my internet connection                ***
*** incessantly so I'll not be able to respond to e-mails                     ***
*** regularly and my work will be somehow irregular.                          ***
*** I'm already trying to switch ISP but it will take handful amount of time. ***