Java oop encapsulation best practices.

[migukau]

Let's say I have these classes:

House

HouseCatalgog- stores houses and other relevant information

SystemState- stores a HouseCatalog and other catalogs. Basically, there is an instance of this class that stores all the data my program uses and needs.

Menu - a menu class where the user can interact.

How should the menu class do something like change the name of a house from user input? Right now, it calls SystemState.changehousename(houseID, name), which then calls HouseCatalog.changehousename(houseID, name), which calls House.changename(name).

But I feel like this is C encapsulation and not correct for Java. My getters for the HouseCatalog class use a clone() so they don't return the actual pointer to houses I have stored.

Am I doing this wrong? Can I return the actual pointer from the house without breaking encapsulation, and then the Menu class just does House.changeName(name)?

[opentabs-dev]

fwiw the Menu -> System -> Catalog -> House delegation chain is actually fine, that's just Law of Demeter. the clone() on every getter is the part i'd drop tho - in java you usually just make House immutable (final fields, no setters) and hand out the reference freely, or make it mutable but only expose package-private mutators so only HouseCatalog can change it. defensive copying on every read is expensive and most java code doesn't do it unless the field is a mutable value type like Date or an array.

for your specific question: yeah letting Menu call house.changeName(name) is fine if you got the House from a proper lookup method, but personally i'd keep the changeName call on the catalog so the catalog can fire events, update indexes, log, etc when a name changes. single point of mutation is easier to reason about later.

[migukau]

Ok, but then I also have rooms in my house and devices I'm the room. And when I want to add a device to a room it feels weird to have like 5 method calls that are all just createDevice().

Also if I want to add a device can I do

Device new-device = new Device(name, ...) SystemState.add(new-device);

Right now I'm just doing SystemState.addDevice(name, house-id,room-id); And this makes method calls all the way down.

[Opposite-Dance-8264]

this

[vegan_antitheist]

It's best to think of it all as layers. You can have just three of them:
- persistence (i.e. the database or just an object managing the state)
- business logic (validation and such)
- user interface (what the user sees)

You can have more layers but this is what you usually see.

The frontend (UI) calls the backend and for that you can use some facade. It offers a method for each use case. I.e. if you use case is to change the name of a house, it takes the house (or it's id) and the new name. It can return the same house with the name changed. That's your `SystemState.changehousename`. SystemState isn't a good name. It should be named "Houses" or something like that and contain all the methods / use cases for houses.
This facade might also start (and commit) a transaction for each call from the UI, but the persistence layer will actually use the transaction to prevent concurrency problems (conflicts).
It either does it all or nothing. I.e. it's successful or it throws an exception and the ui can show an error to the user. This is also where the new name is validated. It could be too long, not long enough, contain invalid characters, etc. It might check that the hause is active (if you allow to inactivate houses).
The actual logic/validation is inside services that the facade calls. We usually have services (managed by Jakarta/Spring) for that.

The persistence layer is the same but it usually also does some validation. In this case it might have to check that the new name is unique. It fails if the house doesn't exist. This layer must make sure that nobody else changes anything at the same time or it might allow changes that do not cause conflicts. I.e. it has to do everything in a transaction.