Awesome to see this stuff getting added in! On a slightly less positive side, I think we need to look more closely at coding style and naming conventions of things in T4 in order to maintain an easy to read / understand code base for the system as a whole. I've been guilty of straying myself a lot, so take this with a grain of salt...
JavascriptManager / impl in particular had a couple little things that we might want to change before release: -) Method names probably don't need to have "Js" embedded in all of them - such as getJs<Foo> - as that should hopefully be implied by the class name of the object already. -) Some of the method names are either confusing or the javadoc doesn't have enough information. (again, I'm just as guilty ) For instance, I have absolutely no idea what getMainJsWidgetAsset / getJsTapestryAsset means just looking at the interface and javadocs provided. -) class level members are almost always declared in the top of class files in the system - not the bottom. -) JavascriptManagerImpl - Sometimes the curly braces (for constructor / method declarations ) are on new lines and sometimes the same line. The system has thus far been putting them on a new line. (though some things may have escaped this) -) Need lots more new lines and whitespace in general. org.apache.tapestry.pageload.PageLoader might be a good example. Or anything else that has mostly been written by Howard. He writes very visually clean / easy to read code. I usually switch between being slightly more verbose / tight when writing javascript code (as it's interpreted and whitespace / new lines do ultimately hurt you) and trying to use lots more whitespace / new lines in java as there is really no reason not to give everything plenty of space since it will be compiled down to byte code that doesn't care about spacing. Again, please don't take it the wrong way as we both know I'm obviously screwing up plenty here and there with the same stuff / other things as well. Just want to be sure that we try and keep T4 up to par even if it isn't the latest and greatest stuff in the tapestry dev world. On Nov 13, 2007 3:22 PM, <[EMAIL PROTECTED]> wrote: > Author: andyhot > Date: Tue Nov 13 12:22:54 2007 > New Revision: 594630 > > URL: http://svn.apache.org/viewvc?rev=594630&view=rev > Log: > test for jsmanager > > Added: > > tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/javascript/ > > tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/javascript/TestJavascriptManagerImpl.java > Modified: > > tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/javascript/JavascriptManagerImpl.java > > Modified: > tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/javascript/JavascriptManagerImpl.java > URL: > http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/javascript/JavascriptManagerImpl.java?rev=594630&r1=594629&r2=594630&view=diff > ============================================================================== > --- > tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/javascript/JavascriptManagerImpl.java > (original) > +++ > tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/javascript/JavascriptManagerImpl.java > Tue Nov 13 12:22:54 2007 > @@ -14,9 +14,10 @@ > > package org.apache.tapestry.javascript; > > -import java.util.List; > import java.util.ArrayList; > +import java.util.List; > > +import org.apache.hivemind.HiveMind; > import org.apache.hivemind.Location; > import org.apache.hivemind.util.URLResource; > import org.apache.tapestry.IAsset; > @@ -25,11 +26,20 @@ > import org.apache.tapestry.util.DescribedLocation; > > /** > + * An implementation that accepts a comma separated String for > + * files, formFiles and widgetFiles. > + * > * @author Andreas Andreou > * @since 4.1.4 > */ > public class JavascriptManagerImpl implements JavascriptManager { > > + public JavascriptManagerImpl() { > + _files = new ArrayList(); > + _formFiles = new ArrayList(); > + _widgetFiles = new ArrayList(); > + } > + > public IAsset getMainJsAsset() > { > return findFirst(_files); > @@ -122,7 +132,7 @@ > private IAsset findAsset(String path, String description) > { > IAsset asset = null; > - if (path!=null) > + if ( !HiveMind.isBlank(path) ) > { > Location location = new DescribedLocation(new URLResource(path), > description); > asset = _assetSource.findAsset(null, path, null, location); > @@ -131,7 +141,7 @@ > } > > private IAsset findFirst(List list) { > - if (list ==null || list.isEmpty()) > + if (list == null || list.isEmpty()) > return null; > else > return (IAsset) list.get(0); > > Added: > tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/javascript/TestJavascriptManagerImpl.java > URL: > http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/javascript/TestJavascriptManagerImpl.java?rev=594630&view=auto > ============================================================================== > --- > tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/javascript/TestJavascriptManagerImpl.java > (added) > +++ > tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/javascript/TestJavascriptManagerImpl.java > Tue Nov 13 12:22:54 2007 > @@ -0,0 +1,94 @@ > +package org.apache.tapestry.javascript; > + > +import java.util.Locale; > + > +import org.apache.tapestry.IAsset; > +import org.apache.tapestry.TestBase; > +import org.apache.tapestry.util.DescribedLocation; > +import org.apache.tapestry.asset.AssetSource; > +import org.apache.hivemind.Resource; > +import org.testng.annotations.Test; > + > +import static org.easymock.EasyMock.*; > + > [EMAIL PROTECTED] > +public class TestJavascriptManagerImpl extends TestBase > +{ > + public void test_null_manager() > + { > + JavascriptManagerImpl impl = createImpl(); > + replay(); > + > + assertNullAndEmpty(impl); > + > + verify(); > + } > + > + public void test_empty_manager() > + { > + JavascriptManagerImpl impl = createImpl("", "", "", "", "", ""); > + replay(); > + > + assertNullAndEmpty(impl); > + > + verify(); > + } > + > + public void test_several_files() > + { > + AssetSource source = newMock(AssetSource.class); > + expectFile(source, "a.js"); > + expectFile(source, "b.js"); > + expectFile(source, "tap"); > + > + replay(); > + > + JavascriptManagerImpl impl = createImpl(source, "a.js, b.js", "", > "", "", "tap", ""); > + assertEquals(impl.getJsAssets().size(), 2); > + assertNotNull(impl.getMainJsAsset()); > + assertNotNull(impl.getJsTapestryAsset()); > + > + verify(); > + } > + > + private void expectFile(AssetSource source, String file) { > + expect(source.findAsset((Resource) isNull(), eq(file), > + (Locale) isNull(), isA(DescribedLocation.class))) > + .andReturn(newMock(IAsset.class)); > + } > + > + private void assertNullAndEmpty(JavascriptManagerImpl impl) { > + assertNull(impl.getJsPath()); > + assertNull(impl.getJsTapestryAsset()); > + assertNull(impl.getJsTapestryPath()); > + assertNull(impl.getMainJsAsset()); > + assertNull(impl.getMainJsFormAsset()); > + assertNull(impl.getMainJsWidgetAsset()); > + assertTrue(impl.getJsAssets().isEmpty()); > + assertTrue(impl.getJsFormAssets().isEmpty()); > + assertTrue(impl.getJsWidgetAssets().isEmpty()); > + } > + > + private JavascriptManagerImpl createImpl(String...params) { > + return createImpl(newMock(AssetSource.class), params); > + } > + > + private JavascriptManagerImpl createImpl(AssetSource source, String... > params) { > + JavascriptManagerImpl impl = new JavascriptManagerImpl(); > + impl.setAssetSource(source); > + > + if (params.length>0) > + impl.setFiles(params[0]); > + if (params.length>1) > + impl.setFormFiles(params[1]); > + if (params.length>2) > + impl.setWidgetFiles(params[2]); > + if (params.length>3) > + impl.setPath(params[3]); > + if (params.length>4) > + impl.setTapestryFile(params[4]); > + if (params.length>5) > + impl.setTapestryPath(params[5]); > + return impl; > + } > +} > > > -- Jesse Kuhnert Tapestry/Dojo team member/developer Open source based consulting work centered around dojo/tapestry/tacos/hivemind. http://blog.opencomponentry.com --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
