Это неизменный класс? - PullRequest
       9

Это неизменный класс?

12 голосов
/ 30 сентября 2010

Я понятия не имею, как должен выглядеть неизменный класс, но я уверен, что это тот класс Я прав? Если нет, укажите, что следует добавить / удалить.

import java.io.Serializable;

public class Triangle implements IShape, Serializable {
    private static final long serialVersionUID = 0x100;

    private Point[] points;

    public Triangle(Point a, Point b, Point c) {
        this.points = new Point[]{a, b, c};
    }

    @Override
    public Point[] getPoints() {
        return this.points;
    }

    @Override
    public boolean equals(Object obj) {
        if (obj == null) return false;
        if (this == obj) return true;
        if (getClass() != obj.getClass()) return false;
        Point[] trianglePoints = ((Triangle) obj).getPoints();
        for (int i = 0; i < points.length; i++){
            if (!points[i].equals(trianglePoints[i])) return false;
        }
        return true;
    }
}

Это поможет?

@Override
    public Point[] getPoints() {
        Point[] copyPoint = {
                new Point(points[0]),
                new Point(points[1]),
                new Point(points[2]),};
        return copyPoint;
    }

Класс очков:

import java.io.Serializable;

public class Point implements Serializable {
    private static final long serialVersionUID = 0x100;

    public int x;
    public int y;
    public int z;

    public Point(int x, int y, int z) {
        this.x = x;
        this.y = y;
        this.z = z;
    }

    public Point(Point that) {
        this.x = that.x;
        this.y = that.y;
        this.z = that.z;
    }

    public boolean equals(Object obj) { 
        // assume this is a typical, safe .equals implementation
        // that compares the coordinates in this instance to the
        // other instance
        return true;
    }
}

Ответы [ 14 ]

17 голосов
/ 30 сентября 2010

Нет, вы можете изменить содержимое массива Points.Если вы хотите сделать его неизменным, попросите получателя раздать копию массива Points, а не оригинал.

попробуйте это:

Triangle triangle = new Triangle(a, b, c);
triangle.getPoints()[1] = null;
System.out.println(Arrays.toString(triangle.getPoints()));

Также Point должен быть неизменнымНикита Рыбак указывает).О том, как копировать массивы, см. Как копировать массив в Java .

10 голосов
/ 30 сентября 2010

Нет, это не так.Вы выставляете Point [] и вызывающая сторона может изменить его содержимое.Кроме того, ваш класс не является окончательным, поэтому кто-то может подорвать его, сделав его подклассом.

8 голосов
/ 30 сентября 2010

Нет, это определенно изменчиво.

Мало того, что вы выставляете фактический массив Point [], вы не копируете защитную копию ( Bloch 2nd ed. , Item 39)Точечные объекты сами по себе, когда принимают их через конструктор.

  1. В массиве Point [] могут быть удалены или добавлены элементы, поэтому он может изменяться.
  2. Вы можете передавать в Point a, b и c, затем вызовите для них setX () или setY (), чтобы изменить их данные после построения.
6 голосов
/ 30 сентября 2010

Закрыть. С одной стороны, неизменный класс должен сделать его поля окончательными, но это не является обязательным.

Однако вы выставляете массив через геттер, и это не является неизменным. Сделайте защитную копию, используя Arrays.copyOf (массив, длина) :

@Override
public Point[] getPoints() {
    return Arrays.copyOf(this.points,this.points.length);
}
4 голосов
/ 30 сентября 2010

Вот что я бы сделал, чтобы сделать этот класс неизменным, с помощью Гуава . Из кода @Override, который вы выложили, я вижу, что IShape, по-видимому, требует Point[] от метода getPoints(), но я игнорирую это для примера, поскольку использование массивов объектов довольно плохая идея, особенно если вы хотите неизменности (поскольку они не могут быть неизменными и все).

public final class Triangle implements IShape, Serializable {
  private final ImmutableList<Point> points;

  public Triangle(Point a, Point b, Point c) {
    this.points = ImmutableList.of(a, b, c);
  }

  public ImmutableList<Point> getPoints() {
    return this.points;
  }

  // ...
}

Point также должно быть больше похоже на:

public final class Point implements Serializable {
  /*
   * Could use public final here really, but I prefer
   * consistent use of methods.
   */
  private final int x;
  private final int y;
  private final int z;

  public Point(int x, int y, int z) {
    this.x = x;
    this.y = y;
    this.z = z;
  }

  // getters, etc.
}
3 голосов
/ 30 сентября 2010

Чтобы быть неизменным классом, недостаточно, чтобы ваши методы обещали не изменять объект.Помимо того, что все поля являются закрытыми, а методы, не допускающие изменения, вы также должны гарантировать, что подклассы имеют одинаковое обещание неизменности.Это включает в себя завершение самого класса и гарантию того, что ссылки на поля никогда не возвращаются.

Короткая, но превосходная обработка этого может быть найдена в этой статье:

http://www.javaranch.com/journal/2003/04/immutable.htm

1 голос
/ 30 сентября 2010

Вам нужно не только предоставить неизменяемую копию встроенного массива, также необходимо убедиться, что объект Point неизменен.

Рассмотрим следующее использование класса Point в стандартном Java API:

Point a = new Point(1,1);
Point b = new Point(1,1);
Point c = new Point(1,1);
Triangle triangle = new Triangle(a, b, c);
System.out.println(Arrays.toString(triangle.getPoints()));
c.setLocation(99,99);
System.out.println(Arrays.toString(triangle.getPoints()));
0 голосов
/ 27 февраля 2014

Пример неизменяемого класса с изменяемым полем:

public  final class  ImmutabilityTest {

    private final int i;
    private final C c1;

    ImmutabilityTest(int i, C c1){
        this.i = i;
        this.c1 = c1;
    }

    public int getI() {
        return i;
    }
    public C getC1() {
        return (C)c1.clone();//If return c1 simply without calling clone then contract of immutable object will break down 
    }

    @Override
    public String toString() {
        return "ImmutabilityTest [i=" + i + ", c1=" + c1 + "]";
    }


    public static void main(String[] args) {

        ImmutabilityTest i1 = new ImmutabilityTest(10, new C(new D("before")));
        System.out.println(i1);
        i1.getC1().getD1().name = "changed";

        System.out.println(i1);

    }

}

class C implements Cloneable{
    D d1;

    public C(D d1) {
        super();
        this.d1 = d1;
    }

    public D getD1() {
        return d1;
    }

    public void setD1(D d1) {
        this.d1 = d1;
    }


    @Override
    public String toString() {
        return "C [d1=" + d1 + "]";
    }

    public C clone(){
        C c = null;
        try {
            c = (C) super.clone();
            c.setD1(c.getD1().clone());// here deep cloning is handled if it is commented it will become shallow cloning
        } catch (CloneNotSupportedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        return c;
    }

}

class D implements Cloneable{
    String name;

    public D(String name) {
        this.name = name;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }


    @Override
    public String toString() {
        return "D [name=" + name + "]";
    }

    public D clone(){
        D d = null;
        try {
            d = (D) super.clone();
        } catch (CloneNotSupportedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        return d;
    }
}
0 голосов
/ 30 сентября 2010

Сама точка не должна быть неизменной, чтобы Треугольник был неизменным.Вам просто нужно сделать много защитных копий, чтобы никто не имел ссылки на объекты Point, хранящиеся в треугольнике.

Кроме того, треугольник abc не должен равняться треугольнику bca (и 4 другим перестановкам)

0 голосов
/ 30 сентября 2010

За исключением предоставления массива (как обычно делают геттеры) и отсутствия final, сериализуемость «проблематична».

Как очень неприятный человек, при десериализации я могу получить еще одну ссылку на внутренний массив. Очевидное исправление для этого:

private void readObject(
    ObjectInputStream in 
) throws ClassNotFoundException, IOException {
    ObjectInputStream.GetField fields = in.readFields();
    this.points = ((Point[])(fields.get("point", null)).clone();
}

Это все еще оставляет проблему points не быть final и выставлять объект без инициализации points (или, что еще хуже, но немного тереетическим, частично инициализированным). То, что вы действительно хотите, это «последовательный прокси», о котором вы можете узнать в интернете ...

Примечание. Если вы внедрили equals, вам также следует реализовать hashCode, возможно toString и, возможно, Comparable.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...