review comments addressed

Author: avinash-anand

src/main/java/io/r2dbc/postgresql/codec/Point.java

@@ -3,11 +3,13 @@
 /**
  * <p>It maps to the point datatype in org.postgresql.</p>
  *
- * <p>This implements a version of java.awt.Point, except it uses double to represent the coordinates.</p>
+ * <p> It uses double to represent the coordinates.</p>
  */
 public class Point {
-    private double x;
-    private double y;
+
+    private final double x;
+
+    private final double y;
 
     /**
      * @param x coordinate
@@ -22,37 +24,30 @@ public double getX() {
         return x;
     }
 
-    public void setX(double x) {
-        this.x = x;
-    }
-
     public double getY() {
         return y;
     }
 
-    public void setY(double y) {
-        this.y = y;
-    }
-
     /**
      * Translate the point by the supplied amount.
      *
      * @param x integer amount to add on the x axis
      * @param y integer amount to add on the y axis
+     * @return - new point with translated values
      */
-    public void translate(int x, int y) {
-        translate((double) x, (double) y);
+    public Point translate(int x, int y) {
+        return translate((double) x, (double) y);
     }
 
     /**
      * Translate the point by the supplied amount.
      *
      * @param x double amount to add on the x axis
      * @param y double amount to add on the y axis
+     * @return - new point with translated values
      */
-    public void translate(double x, double y) {
-        this.x += x;
-        this.y += y;
+    public Point translate(double x, double y) {
+        return new Point(this.x + x, this.y + y);
     }
 
     /**
@@ -61,40 +56,30 @@ public void translate(double x, double y) {
      * @param x integer coordinate
      * @param y integer coordinate
      */
-    public void move(int x, int y) {
-        setLocation(x, y);
+    public Point move(int x, int y) {
+        return setLocation(x, y);
     }
 
     /**
      * Moves the point to the supplied coordinates.
      *
      * @param x double coordinate
      * @param y double coordinate
+     * @return - new point with provided coordinates
      */
-    public void move(double x, double y) {
-        this.x = x;
-        this.y = y;
+    public Point move(double x, double y) {
+        return new Point(x, y);
     }
 
     /**
-     * Moves the point to the supplied coordinates. refer to java.awt.Point for description of this.
+     * Moves the point to the supplied coordinates.
      *
      * @param x integer coordinate
      * @param y integer coordinate
-     * @see java.awt.Point
-     */
-    public void setLocation(int x, int y) {
-        move((double) x, (double) y);
-    }
-
-    /**
-     * Moves the point to the supplied java.awt.Point refer to java.awt.Point for description of this.
-     *
-     * @param p Point to move to
-     * @see java.awt.Point
+     * @return - return new Point with these coordinates
      */
-    public void setLocation(java.awt.Point p) {
-        setLocation(p.x, p.y);
+    public Point setLocation(int x, int y) {
+        return move((double) x, (double) y);
     }
 
     public boolean equals(Object obj) {

src/main/java/io/r2dbc/postgresql/codec/PointCodec.java

@@ -7,6 +7,7 @@
 import io.r2dbc.postgresql.type.PostgresqlObjectId;
 import io.r2dbc.postgresql.util.Assert;
 import io.r2dbc.postgresql.util.ByteBufUtils;
+import reactor.util.annotation.Nullable;
 
 import static io.r2dbc.postgresql.message.Format.FORMAT_BINARY;
 import static io.r2dbc.postgresql.type.PostgresqlObjectId.POINT;
@@ -21,9 +22,8 @@ public class PointCodec extends AbstractCodec<Point> {
     }
 
     @Override
-    boolean doCanDecode(PostgresqlObjectId type, Format format) {
+    boolean doCanDecode(PostgresqlObjectId type, @Nullable Format format) {
         Assert.requireNonNull(type, "type must not be null");
-        Assert.requireNonNull(format, "format must not be null");
 
         return POINT == type;
     }
@@ -49,8 +49,7 @@ Point doDecode(ByteBuf buffer, PostgresqlObjectId dataType, Format format, Class
     @Override
     Parameter doEncode(Point value) {
         Assert.requireNonNull(value, "value must not be null");
-        String encodedValue = "(" + value.getX() + "," + value.getY() + ")";
-        return create(POINT, FORMAT_BINARY, () -> ByteBufUtils.encode(this.byteBufAllocator, encodedValue));
+        return create(POINT, FORMAT_BINARY, () -> ByteBufUtils.encode(this.byteBufAllocator, value.toString()));
     }
 
     @Override

src/test/java/io/r2dbc/postgresql/codec/PointCodecTest.java

@@ -20,19 +20,13 @@ class PointCodecTest {
     @Test
     void constructorNoByteBufAllocator() {
         assertThatIllegalArgumentException().isThrownBy(() -> new PointCodec(null))
-                .withMessage("byteBufAllocator must not be null");
+            .withMessage("byteBufAllocator must not be null");
     }
 
     @Test
     void doCanDecodeNoType() {
         assertThatIllegalArgumentException().isThrownBy(() -> new PointCodec(TEST).doCanDecode(null, FORMAT_BINARY))
-                .withMessage("type must not be null");
-    }
-
-    @Test
-    void doCanDecodeNoFormat() {
-        assertThatIllegalArgumentException().isThrownBy(() -> new PointCodec(TEST).doCanDecode(POINT, null))
-                .withMessage("format must not be null");
+            .withMessage("type must not be null");
     }
 
     @Test
@@ -47,19 +41,19 @@ void doCanDecode() {
     @Test
     void doDecodeNoByteBuf() {
         assertThatIllegalArgumentException().isThrownBy(() -> new PointCodec(TEST).doDecode(null, POINT, FORMAT_BINARY, Point.class))
-                .withMessage("byteBuf must not be null");
+            .withMessage("byteBuf must not be null");
     }
 
     @Test
     void doDecodeNoType() {
         assertThatIllegalArgumentException().isThrownBy(() -> new PointCodec(TEST).doDecode(TEST.buffer(), POINT, FORMAT_BINARY, null))
-                .withMessage("type must not be null");
+            .withMessage("type must not be null");
     }
 
     @Test
     void doDecodeNoFormat() {
         assertThatIllegalArgumentException().isThrownBy(() -> new PointCodec(TEST).doDecode(TEST.buffer(), POINT, null, Point.class))
-                .withMessage("format must not be null");
+            .withMessage("format must not be null");
     }
 
     @Test
@@ -69,28 +63,28 @@ void doDecode() {
         String encodedValue = "(1.12,2.12)";
 
         assertThat(codec.doDecode(ByteBufUtils.encode(TEST, encodedValue), POINT, FORMAT_BINARY, Point.class))
-                .isEqualTo(point);
+            .isEqualTo(point);
     }
 
     @Test
     void doEncodeNoValue() {
         assertThatIllegalArgumentException().isThrownBy(() -> new PointCodec(TEST).doEncode(null))
-                .withMessage("value must not be null");
+            .withMessage("value must not be null");
     }
 
     @Test
     void doEncode() {
         PointCodec codec = new PointCodec(TEST);
 
         ParameterAssert.assertThat(codec.doEncode(new Point(1.12, 2.12)))
-                .hasFormat(FORMAT_BINARY)
-                .hasType(POINT.getObjectId())
-                .hasValue(encode(TEST, "(1.12,2.12)"));
+            .hasFormat(FORMAT_BINARY)
+            .hasType(POINT.getObjectId())
+            .hasValue(encode(TEST, "(1.12,2.12)"));
     }
 
     @Test
     void encodeNull() {
         ParameterAssert.assertThat(new PointCodec(TEST).encodeNull())
-                .isEqualTo(new Parameter(FORMAT_BINARY, POINT.getObjectId(), NULL_VALUE));
+            .isEqualTo(new Parameter(FORMAT_BINARY, POINT.getObjectId(), NULL_VALUE));
     }
 }