2

I was taught that an object should know how to do everything with itself. So I've built an application trying to keep fields private and have a lot of methods like these:

  • DisplayGraphically()
  • DisplayAsText()
  • WriteToFile()
  • Apply()
  • Edit() (shows a dialog box to edit it)
  • ErrorCheck()

Now I want to save the data to another file format whose structure doesn't closely correspond to my object structure. It would feel easy if all the data was public. Then a single method could collect all the various bits of data from the objects and reorganize it to suit the file format. But this means exposing private fields (through getters). On the other hand, that's what an API would provide and surely doing this job through an API would be quite clean.

To sum up, which is better in what cases?

  1. Fields are private. Objects have lots of methods to deal with many different uses.
  2. Fields have public getters and setters which maintain correct internal state. New functionality that involves many objects can be encapsulated in its own class and doesn't require any modification of the existing code.
Improve this question
3
  • 3
    Take a step back. Perhaps I/O should not be ingrained into your object to begin with? It might be better if you create something like a "persistence strategy" that knows how to take an object of type X and convert it to format Y. This does not really answer the question as written, but it might be a better design.
    – user22815
    Sep 27, 2014 at 2:28
  • there is also such a thing as SRP single responsible principle. which goes against "should know how to do everything with it self" I would take that with a grain of salt. everything is a balance. nobody said software development is easy :)
    – Esben Skov Pedersen
    Sep 27, 2014 at 8:27
  • A "reorganize" function implemented outside your class would need to use some accessors that return the necessary data values at a sufficient granularity for it to do its job. This does not necessarily correspond one-to-one with the fields of your class.
    – David K
    Sep 27, 2014 at 12:26

2 Answers 2

Reset to default
3

You need to separate your data objects clearly from other application's classes, following the single responsibility principle. Most of the methods from your example should not belong to your data objects, they should belong to controller classes or other application classes, because the responsibility of your data objects should only be, well, to hold or manage their data, and not to interact with the file system or the user interface. If you start to restructure your application that way, your data objects (and only the data objects) will have most of their fields public accessible already.

For example, a method like DisplayAsText() which actually displays your object on screen does clearly not belong into any of your data objects. Instead, a method like ToString() could be part of your data objects, which is called from a method DisplayAsText() beeing part of a completely different class somewhere else in your application. A method like ErrorCheck could call a lot of small validation methods of your data objects telling if some state is right or wrong, and display the error, which means, the small validation methods should be part of your data objects, and ErrorCheck itself clearly not. And a method like Edit() will typically need complete read/write access to every data field of your data objects, so you should already have this available when starting to implement your SaveToNewFormat method.

Improve this answer
7
  • SRP looks like it will lead to an explosion in the number of classes - a new FileWriter, NewFormatConverter, Editor, etc for each existing data class. Is that something to be concerned about? Same amount of code but separated into several times as many classes.
    – user1318499
    Sep 27, 2014 at 13:17
  • 1
    @user1318499: you should more be converned about having too few classes (with too many lines of code) than too many if there are so many functionalities in your application. And if applying the SRP to your code leads to "an explosion in the number of classes", then there is something wrong with your code, not with the SRP.
    – Doc Brown
    Sep 27, 2014 at 15:04
  • This answer shows how the overhyped SRP leads to anemic classes.
    – user949300
    Sep 27, 2014 at 20:51
  • @Doc Brown. Just to be sure I'm doing this right. Since most data classes have quite different data (various combinations of text, pictures, lists, doubles, integers, etc), and assuming that's an OK design, then does each data class need its own unique Editor class which only has the responsibility of letting the user edit that class's data? All of these would implement a common IEditable interface.
    – user1318499
    Sep 28, 2014 at 0:19
  • @user1318499: well, I don't know the structure or the use cases of your application, but your comment sounds you are confusing the internal structure of your data with the viewpoint of the user. Both should be mostly independent. An Editor should allow to edit exactly the data which is relevant to a certain use case, which can be very different from how the data objects are structured.
    – Doc Brown
    Sep 28, 2014 at 7:17
0

Fields are private. Objects have lots of methods to deal with many different uses.

Keep fields private (getters & setters are evil), but also reduce the specific things each object is doing by using other specialized objects.

To give an example no object should have both these methods

DisplayGraphically()
DisplayAsText()

They should have at most a single display method. It is not the objects job to figure out where they are displaying to. Instead you should have a Display object that does the grunt work and can either be a graphic display or a console display. This class should have a clear interface that the your object expects and understand.

So instead of the two different messages above replace with 

myobj.display_myself(display_to_use)

Your object knows only how to use this Display object to display itself, most likely by simply calling the Display object with internal state of the object.

It is the display object itself knows how to display that information. You could have a thousand types of Display classes but your original object still only needs one method.

So say you are saving your objects data to a DB. Your object shouldn't care. It just has

myobj.persist_myself(persistence_to_use)

where persistence_to_use can be an object that knows how to write to a database, or an object that knows about files, or an object that knows about back up tape disks etc.

You want to completely change the file format you use to persist your data. Great, your object doesn't care a bit. You simply create a new Persistence object that does what ever new fancy file system stuff you want and pass that object instead of your old persistence object. myobj doesn't even notice this change.

Improve this answer
11
  • Your object knows only how to use this Display object to display itself The display code should depend on the object, not the other way around. Drawing a circle is one of infinitely many things you may choose to do or not to do with it, and a circle is not defined by whether its ability to accept some abstract Display to draw itself to. The inclusion of non-essential operations is a subtle violation of the Interface Segregation Principle (because not everyone needs to draw circles) and Dependency Inversion Principle (because changes to the higher level display API breaks your circle).
    – Doval
    Oct 2, 2014 at 18:45
  • +1 for "getters and setters are evil". When designing, consider what is most likely to change, the Display (hopefully a standard, abstracted component like a Canvas, JPanel, GC) or a business object which marketing is constantly adding fields to?
    – user949300
    Oct 2, 2014 at 19:59
  • @Doval If the display code depends on the object you break encapsulation, because the display only knows what points to draw by ripping the data out of the Circle object ie central point, radius, pi etc. You have just moved the behaviour "Draw a circle" from the Circle to the Display object making the Display object fat with knowledge about circles. ISP does not say that you should have no method unless everyone uses it, it says no one should have to depend on a method or interface they don't want to use. Drawing a circle is a behaviour that exists in the system and thus makes sense in Circle
    – Cormac Mulhall
    Oct 3, 2014 at 8:36
  • @CormacMulhall You're going to have to know the circle's center point and radius no matter where you put the display code. A circle type should contain enough information to determine its geometry and no more. Once you start breaking that rule, you enter a slippery slope of additions. Today you want to draw the circle. Tomorrow you're adding a method to accept an IPersister to store it on a database. Next week you're adding a method to send it over the network. Next month you're adding a method to encode it to JSON. Before you know it, no one but you will want to use your circle type.
    – Doval
    Oct 3, 2014 at 11:18
  • 1
    @Doval If you try and produce a universal circle object that can work in any context you are doomed to failure, because a "circle" in one context has different behaviour to a "circle" in another context and OOD is about modelling behaviour not any notion of Platonic forms. The behaviour determine your geometry could be next to useless in say a graphics package. You produce anemic objects that encapsulate no relevant behaviour and then simply shift the actual behaviour some where else in the name of object reuse.
    – Cormac Mulhall
    Oct 3, 2014 at 12:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.