You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gary Gregory <ga...@gmail.com> on 2013/11/28 13:01:17 UTC

RE: svn commit: r1546339 - /commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmConstants.java

This backward IMO and an anti-pattern: an interface should only define a contract for a service, not constants. 

Gary

-------- Original message --------
From: ebourg@apache.org 
Date:11/28/2013  05:41  (GMT-05:00) 
To: commits@commons.apache.org 
Subject: svn commit: r1546339 -
  /commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmConstants.java 

Author: ebourg
Date: Thu Nov 28 10:41:37 2013
New Revision: 1546339

URL: http://svn.apache.org/r1546339
Log:
Turn PnmConstants into an interface and make it package private

Modified:
    commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmConstants.java

Modified: commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmConstants.java
URL: http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmConstants.java?rev=1546339&r1=1546338&r2=1546339&view=diff
==============================================================================
--- commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmConstants.java (original)
+++ commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmConstants.java Thu Nov 28 10:41:37 2013
@@ -16,21 +16,18 @@
  */
package org.apache.commons.imaging.formats.pnm;

-public final class PnmConstants {
-    public static final byte PNM_PREFIX_BYTE = 0x50; // P
+interface PnmConstants {
+    byte PNM_PREFIX_BYTE = 0x50; // P

-    public static final byte PBM_TEXT_CODE = 0x31; // Textual Bitmap
-    public static final byte PGM_TEXT_CODE = 0x32; // Textual GrayMap
-    public static final byte PPM_TEXT_CODE = 0x33; // Textual Pixmap
-    public static final byte PGM_RAW_CODE = 0x35; // RAW GrayMap
-    public static final byte PBM_RAW_CODE = 0x34; // RAW Bitmap
-    public static final byte PPM_RAW_CODE = 0x36; // RAW Pixmap
-    public static final byte PAM_RAW_CODE = 0x37; // PAM Pixmap
+    byte PBM_TEXT_CODE = 0x31; // Textual Bitmap
+    byte PGM_TEXT_CODE = 0x32; // Textual GrayMap
+    byte PPM_TEXT_CODE = 0x33; // Textual Pixmap
+    byte PGM_RAW_CODE = 0x35; // RAW GrayMap
+    byte PBM_RAW_CODE = 0x34; // RAW Bitmap
+    byte PPM_RAW_CODE = 0x36; // RAW Pixmap
+    byte PAM_RAW_CODE = 0x37; // PAM Pixmap

-    public static final byte PNM_SEPARATOR = 0x20; // Space
-    public static final byte PNM_NEWLINE = 0x0A; // "usually a newline"
-                                                 // (http://netpbm.sourceforge.net/doc/pbm.html)
-    
-    private PnmConstants() {
-    }
+    byte PNM_SEPARATOR = 0x20; // Space
+    byte PNM_NEWLINE = 0x0A; // "usually a newline"
+                             // (http://netpbm.sourceforge.net/doc/pbm.html)
}



Re: svn commit: r1546339 - /commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmConstants.java

Posted by sebb <se...@gmail.com>.
On 28 November 2013 12:34, Emmanuel Bourg <eb...@apache.org> wrote:
> Le 28/11/2013 13:01, Gary Gregory a écrit :
>> This backward IMO and an anti-pattern: an interface should only define a contract for a service, not constants.
>
> I know, but that's more idiomatic than a class with only public static
> final fields.

Why?
That seems like quite a sensible use of a class; definitely better
than using an interface.

In any case, the intention needs to be documented in the source file.
We only release source files; documentation that is in SCM logs or
mailing lists is useless.

> The class is private now, so it doesn't matter much. We can improve that
> as we like later.

Surely an interface is effectively public?

> Emmanuel
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1546339 - /commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmConstants.java

Posted by Emmanuel Bourg <eb...@apache.org>.
Le 28/11/2013 13:01, Gary Gregory a écrit :
> This backward IMO and an anti-pattern: an interface should only define a contract for a service, not constants. 

I know, but that's more idiomatic than a class with only public static
final fields.

The class is private now, so it doesn't matter much. We can improve that
as we like later.

Emmanuel


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org