[richfaces-svn-commits] JBoss Rich Faces SVN: r11996 - in trunk/ui/editor/src/main/java/org/richfaces: convert/seamtext and 2 other directories.
richfaces-svn-commits at lists.jboss.org
richfaces-svn-commits at lists.jboss.org
Tue Dec 23 11:08:35 EST 2008
Author: nbelaevski
Date: 2008-12-23 11:08:35 -0500 (Tue, 23 Dec 2008)
New Revision: 11996
Modified:
trunk/ui/editor/src/main/java/org/richfaces/component/UIEditor.java
trunk/ui/editor/src/main/java/org/richfaces/convert/seamtext/DefaultSeamTextConverter.java
trunk/ui/editor/src/main/java/org/richfaces/convert/seamtext/SeamTextConverterFactory.java
trunk/ui/editor/src/main/java/org/richfaces/renderkit/EditorRendererBase.java
trunk/ui/editor/src/main/java/org/richfaces/renderkit/resources/EditorHTMLRenderer.java
Log:
Editor code review results committed
Modified: trunk/ui/editor/src/main/java/org/richfaces/component/UIEditor.java
===================================================================
--- trunk/ui/editor/src/main/java/org/richfaces/component/UIEditor.java 2008-12-23 16:06:54 UTC (rev 11995)
+++ trunk/ui/editor/src/main/java/org/richfaces/component/UIEditor.java 2008-12-23 16:08:35 UTC (rev 11996)
@@ -48,6 +48,7 @@
/** Id suffix of textarea which used as target element for tinyMCE scripts*/
public static final String EDITOR_TEXT_AREA_ID_SUFFIX = "TextArea";
+ //TODO nick - it's a bad idea to create converterFactory instance for each component
private SeamTextConverterFactory converterFactory = new SeamTextConverterFactory();
public abstract void setType(String type);
@@ -142,6 +143,8 @@
if(isUseSeamText() && converter == null) {
return converterFactory.getConverter();
}
+
+ //TODO nick - check that converter is instance of SeamTextConverter if we use SeamText
return converter;
}
Modified: trunk/ui/editor/src/main/java/org/richfaces/convert/seamtext/DefaultSeamTextConverter.java
===================================================================
--- trunk/ui/editor/src/main/java/org/richfaces/convert/seamtext/DefaultSeamTextConverter.java 2008-12-23 16:06:54 UTC (rev 11995)
+++ trunk/ui/editor/src/main/java/org/richfaces/convert/seamtext/DefaultSeamTextConverter.java 2008-12-23 16:08:35 UTC (rev 11996)
@@ -51,10 +51,11 @@
/** The converter id for this converter. */
public static final String CONVERTER_ID = DefaultSeamTextConverter.class.getName();
-
+ //TODO nick - let everybody create instance of this class
private DefaultSeamTextConverter() {
}
+ //TODO nick - this class is not necessary
private static class DefaultConverterHolder {
private static final SeamTextConverter INSTANCE = new DefaultSeamTextConverter();
}
@@ -66,7 +67,7 @@
public Object getAsObject(FacesContext context, UIComponent component,
String value) {
try {
-
+ //TODO nick - value can be null here - see JavaDoc
Reader r = new StringReader(value);
HtmlSeamTextLexer lexer = new HtmlSeamTextLexer(r);
HtmlSeamTextParser parser = new HtmlSeamTextParser(lexer);
Modified: trunk/ui/editor/src/main/java/org/richfaces/convert/seamtext/SeamTextConverterFactory.java
===================================================================
--- trunk/ui/editor/src/main/java/org/richfaces/convert/seamtext/SeamTextConverterFactory.java 2008-12-23 16:06:54 UTC (rev 11995)
+++ trunk/ui/editor/src/main/java/org/richfaces/convert/seamtext/SeamTextConverterFactory.java 2008-12-23 16:08:35 UTC (rev 11996)
@@ -44,9 +44,11 @@
public SeamTextConverter createSeamTextConverter() {
try {
-
+ //TODO nick - use ThreadCCL
Class clazz = Class.forName("org.richfaces.convert.seamtext.DefaultSeamTextConverter");
Method method = clazz.getMethod("getInstance");
+
+ //TODO nick - static method doesn't need any argument, better pass null
return (SeamTextConverter) method.invoke(clazz);
} catch(Exception e) {
Modified: trunk/ui/editor/src/main/java/org/richfaces/renderkit/EditorRendererBase.java
===================================================================
--- trunk/ui/editor/src/main/java/org/richfaces/renderkit/EditorRendererBase.java 2008-12-23 16:06:54 UTC (rev 11995)
+++ trunk/ui/editor/src/main/java/org/richfaces/renderkit/EditorRendererBase.java 2008-12-23 16:08:35 UTC (rev 11996)
@@ -82,6 +82,7 @@
* @param value - component value
* @return converted to String model value
*/
+ //TODO nick - do we really need this method?
protected String getConvertedStringValue(FacesContext context,
UIEditor component, Object value) {
return InputUtils.getConvertedStringValue(context, component, value);
@@ -151,6 +152,8 @@
InternetResource resource = getResource(resourceName);
String resourceUri = resource.getUri(context, null);
String suffix = resourceUri.substring(resourceUri.indexOf(resourceName) + resourceName.length());
+
+ //TODO nick - I doubt suffix can be null
if(suffix == null){
suffix = "";
}
@@ -176,6 +179,8 @@
try {
ClassLoader loader = Thread.currentThread()
.getContextClassLoader();
+
+ //TODO nick - use org.ajax4jsf.resource.util.URLToStreamHelper.urlToStream(URL)
InputStream is = loader.getResourceAsStream(configName
+ ".properties");
if (is == null) {
@@ -184,6 +189,8 @@
+ configName
+ "' was not found in class path");
}
+
+ //TODO nick - streams should be closed, in finally block
parameters.load(is);
writer.writeText(this.convertProperties(parameters), null);
writer.writeText(";\n", null);
@@ -204,6 +211,7 @@
* @param component - Editor component instance
* @throws IOException
*/
+ //TODO nick - merge with writeEditorConfigurationParameters() method
public void writeEditorCustomPluginsParameters(FacesContext context,
UIEditor component) throws IOException {
ResponseWriter writer = context.getResponseWriter();
@@ -325,6 +333,8 @@
+ ScriptUtils.toScript(component.getHeight())
+ ";\n", null);
}
+
+ //TODO nick - use local variables
if (component.getOninit() != null && component.getOninit().length() > 0) {
writer.writeText("tinyMceParams.oninit = function (event) {\n"
+ component.getOninit() + "\n" + "};\n", null);
@@ -405,6 +415,7 @@
* @return true if needed or false if only target textarea should be rendered
*/
public boolean shouldRenderTinyMCE(UIEditor component) {
+ //TODO nick - use TINY_MCE_DISABLED_MODE.equalsIgnoreCase(component.getViewMode())
if (component.getViewMode() != null
&& component.getViewMode().equalsIgnoreCase(
TINY_MCE_DISABLED_MODE)) {
Modified: trunk/ui/editor/src/main/java/org/richfaces/renderkit/resources/EditorHTMLRenderer.java
===================================================================
--- trunk/ui/editor/src/main/java/org/richfaces/renderkit/resources/EditorHTMLRenderer.java 2008-12-23 16:06:54 UTC (rev 11995)
+++ trunk/ui/editor/src/main/java/org/richfaces/renderkit/resources/EditorHTMLRenderer.java 2008-12-23 16:08:35 UTC (rev 11996)
@@ -117,7 +117,11 @@
int total = 0;
BufferedReader br = new BufferedReader(new InputStreamReader(in));
+
+ //TODO nick - why we need BufferedWriter?
BufferedWriter bw = new BufferedWriter(new OutputStreamWriter(out));
+
+ //TODO nick - cannot be changed in runtime, cache in private field
String scriptSuffix = getSriptMappingSuffix();
String cssSuffix = getCssMappingSuffix();
@@ -156,6 +160,7 @@
total += line.getBytes().length;
}
} finally {
+ //TODO nick - close always flushes, no need to do that explicitly
bw.flush();
br.close();
bw.close();
More information about the richfaces-svn-commits
mailing list