About privacy leak

[Ngml Bishop]

Hello everybody.
i hope it's not inappropriate to create a new topic frequently when you got a new question. but believe i'm not that kind of guy who asks any question which can be answered by a simple googling.
it is really making me nuts because i can't figure out where the problem is.
so in the book that i'm studying it says that when creating a copy constructor, or an accessor and mutator, which is required to point to a reference of class types (except for the immutable objects) it is better to use an "anonymous object" instead of using the reference of the object. so that it can not be used to change the value of a private instance to an illegal amount.
for example:

public Date getBirthDate() 
{
return new Date(born); //Safe
} 

public Date getBirthDate() //Unsafe
{
return born;
}

so i took the author's advice. the problem is in one of the projects in this book which i'm trying to modify it, i'm trying to use this kind of "getter" method to access one of private instances to use a "setter" method on it and change a variable's value. so when i do this. it does not do anything. i mean there's no error, but the method seems to perform no action at all and the value remains the same. but as soon as i use the unsafe version of the getter method, it works perfectly fine. and it kind of says "screw your author's advice".
so if you can figure out with this information that i basically misunderstood somethings about java, i'll be happy to use your advice; otherwise tell me to upload the project for you because it's too long for this environment and i don't know which part of it is less important to cut.
thank you.

[Tony Docherty]

Before answering your question do you understand why the author gave the advice he/she did ie do you understand why one of these methods is safer than the other?

[Ngml Bishop]

well as i understand using the unsafe version makes two variable to be used as a name for the same reference. so any change in the first variable cause the same change for the other one. this may be used to intentionally change the first value to something that is not acceptable for the program or even for human logic. for example in an example in the book: it was used to change the "date of birth" (a private instance) so that it is after the date of death,like ("September", 1, 3000), which is insane.

[Tony Docherty]

So returning that actual Date object (or any mutable object) is dangerous because it can be changed by another class. However there are times you want to be able to change the date so you have a setDate() method which takes a Date object verifies it's value makes sense and then sets the instances private Date object to the same value.

Now without seeing the code it's not possible to say exactly what is going wrong but from your description I suspect you are getting an object, changing it, calling the setter but not copying the contents of the changed object to the original object. If this isn't the case (or my description makes no sense to you) please post the setter method and a code snippet showing how you are using it.

[Ngml Bishop]

thanks for taking time, tony.
its supposed to simulate a pizza order class
this is the definition of the setter method

public void setCheese(int cheese)
    {
        if (cheese < 0)
        {
            System.out.println("Error: Negative cheese or pepperoni or ham number.");
            System.exit(0);
        }
        else
            this.cheese = cheese;
    }

this is my getter for the pizza:

public Pizza getPizzaA()
    {
        if (this.pizzaA == null)
            return null;
        else
            return pizzaA; //new Pizza(pizzaA) (safe version)- There is the confusing thing, i used the shallow version
    }

this is the calculation of a single pizza:

public double calcCost()
    {
        int initialCost;
        if (size == 's')
            initialCost = 10;
        else if (size == 'm')
            initialCost = 12;
        else
            initialCost = 14;
        return initialCost + ((cheese+pepperoni+ham)*2);
    }

and finally this is the calculation of the whole order:

public double calcTotal() //calculate the total order price
    {
        if (numPizzas ==1)
            return (pizzaA.calcCost()); 
        else if (numPizzas == 2)
            return (pizzaA.calcCost()+pizzaB.calcCost());
        else
            return (pizzaA.calcCost()+pizzaB.calcCost()+pizzaC.calcCost());
    }

and here is the important part of main method

        PizzaOrder orderB = new PizzaOrder(orderA);

        orderB.getPizzaA().setCheese(3);//This is the setter method that is supposed to change the number of cheese toppings
        double total2 = orderB.calcTotal(); //orderB object bill - total2

sorry for the wall of text. i posted everything that i suspected that might be important to know.
thanks

[Bear Bibeault]

How do you feel that this:

public Pizza getPizzaA()  
    {  
        if (this.pizzaA == null)  
            return null;  
        else  
            return pizzaA; //new Pizza(pizzaA) (safe version)- There is the confusing thing, i used the shallow version  
    }  

is any different than

public Pizza getPizzaA()  
    {  
        return this.pizzaA;  
    }

?

[Bear Bibeault]

But regardless of that, if you are trying to do this:

orderB.getPizzaA().setCheese(3)

then you don't want a "safe" copy of the pizza, as you are trying to change it. You'd only make a "safe" copy when you don't want it to be changed.

[Ngml Bishop]

Bear Bibeault wrote:How do you feel that this:

public Pizza getPizzaA()  
    {  
        if (this.pizzaA == null)  
            return null;  
        else  
            return pizzaA; //new Pizza(pizzaA) (safe version)- There is the confusing thing, i used the shallow version  
    }  

is any different than

public Pizza getPizzaA()  
    {  
        return this.pizzaA;  
    }

?

you are right. i did that because the book asked this: public Pizza getPizza1()— returns the first pizza in the order or null if pizza1 is not set.
i suppose they both do the same,it was kind of, i don't know,self insurance maybe, i'm a beginner buddy, but i don't think that's the point of my question

[Ngml Bishop]

Bear Bibeault wrote:But regardless of that, if you are trying to do this:

orderB.getPizzaA().setCheese(3)

then you don't want a "safe" copy of the pizza, as you are trying to change it. You'd only make a "safe" copy when you don't want it to be changed.

but as i learned, the book specifically states that "do not make the mistake of returning a reference of a class type in an accessor method". and you know what? it makes sense.
because if i return an anonymous object of the class type, then i only can change it with a qualified setter method that won't allow illegal values to be set. right? so i don't think that using the safe version of this kind of accessor is a situational rule. because with your saying the reference can be evaluated to set to another variable. this will make multiple names for the same reference.
i'm sorry for i'm arguing about it, because to be honest i'm not convinced that this is the way to do it

[Bear Bibeault]

Ngml Bishop wrote:i'm a beginner buddy, but i don't think that's the point of my question

That doesn't matter. CodeRanch is a learning site and when poor code is encountered, it's usually going to be called out.

i'm sorry for i'm arguing about it, because to be honest i'm not convinced that this is the way to do it

I really don't know what you are trying to say here.

Regardless, if you return a copy of the original object, and then make changes to the copy, the changes will not be reflected in the original object. That's all simple enough.

[Tony Docherty]

What you have to ask yourself is if you want to set the cheese on a pizza does it make more sense to do it via the order object or via the Pizza object.

If having access to the actual Pizza object that has been added to the order seems a more natural way of doing it then getting the actual object from the order rather than a copy maybe a valid thing to do. The other thing to consider is can you change the Pizza object in a way that will make the order object invalid, if not then returning then actual Pizza object is safe.

[Ngml Bishop]

but my main method is in a separate class, and my pizza object is a private instance, so how can i access to it without an accessor?

[Rico Felix]

I think it would help to think in terms of a real life situation, where you go into a pizza outlet and you order a type of pizza from the menu... After you choose the type of pizza you want, you make customizations to a specific instance of the pizza without affecting any other instance or the generalized type that you choose from which acts as the base template... You can probably simulate it as follows

Types to choose from:

public enum PizzaType
{
	TYPE_A(2.99),
	TYPE_B(3.99),
	TYPE_C(5.99);

	private double cost;

	private PizzaType(double cost) {
		this.cost = cost;
	}

	public double getCost() {
		return cost;
	}
}

Representation of an instance after getting chosen type:

public class Pizza
{
	private int extraCheese;
	private double cost;

	private Pizza(PizzaType type) {
		cost = type.getCost();
	}

	public static Pizza getPizza(PizzaType type) {
		return new Pizza(type);
	}

	public void addCheese(int extraCheese) {
		if (extraCheese < 0) {  
            System.out.println("Error: Negative extraCheese number."); 
            return;  
        }
        for (int i = 0; i < extraCheese; i++) {
        	cost += 0.9;
        }
        this.extraCheese += extraCheese;
	}

	public double getCost() {
		return cost;
	}
}

Purchase order representation:

public class PizzaOrder
{
	java.util.List<Pizza> order;
	private double totalCost;

	public Pizza getPizza(PizzaType type) {
		Pizza pizza = Pizza.getPizza(type);
		if (order == null) {
			order = new java.util.ArrayList<>();
		}
		order.add(pizza);
		return pizza;
	}

	public double getTotalCost() {
		for (Pizza pizza : order) {
			totalCost += pizza.getCost();
		}
		return totalCost;
	}

}

Simulator:

public class Main
{
	public static void main(String[] args) {
		PizzaOrder po = new PizzaOrder();
		po.getPizza(PizzaType.TYPE_A).addCheese(1);
		po.getPizza(PizzaType.TYPE_B).addCheese(3);
		po.getPizza(PizzaType.TYPE_A).addCheese(7);

		System.out.printf("Total cost $%.2f\n", po.getTotalCost());
	}
}

By designing this way there is no violation of privacy because instance is a distinct copy and a change to one instance doesn't affect the other...

[Rico Felix]

But since you are working with examples in a book, all you have to keep in mind is, if you return a reference to an object using a copy constructor then changes to that returned reference would never reflect on the original copy...

[Tony Docherty]

Rico Felix wrote:By designing this way there is no violation of privacy because instance is a distinct copy and a change to one instance doesn't affect the other...

And also in the example shown changing a Pizza instance doesn't impact on the validity of the PizzaOrder object.
In this example, the total price is recalculated each time by asking each Pizza instance what its price is so any changes to a Pizza instance will reflect in the total price. If, however the PizzaOrder object had a totalPrice field which it updated whenever a new Pizza was added to the order then changes to a Pizza instance wouldn't be reflected in the totalPrice of the order and so the PizzaOrder instance would no longer be in a valid state.

If you allow member objects to be changed by external code you must make sure the rest of the classes code allows for external changes.

[J. Kevin Robbins]

Ngml Bishop wrote:

            System.exit(0);

Be very, very cautious about using this. It doesn't just cause the application to exit, it shuts down the entire JVM. It sometimes makes sense for command line tools, but it would be very unusual to use it in a web or desktop application.

[Dieter Quickfend]

J. Kevin Robbins wrote:

Ngml Bishop wrote:

            System.exit(0);

Be very, very cautious about using this. It doesn't just cause the application to exit, it shuts down the entire JVM. It sometimes makes sense for command line tools, but it would be very unusual to use it in a web or desktop application.

Indeed. If you haven't learned about throwing exceptions, it would perhaps be beneficial to use "return;" instead.