How should classes be structured internally as regards methods and their use of instance variables?

By this criterion location and totalGas seem fine -- a real-world car does indeed have a location, and some amount of gas in it, as part of its real-world state. The others are more dubious destination would be fine if you were representing the car at various spots during a trip, or a leg of the trip -- at any given time there would be a present spot, and a destination towards which the car is traveling. But judging from your code that's not what you're doing: the destination instantly becomes the location if gas is sufficient, so you're using a simplified model of reality where the car is only represented as being in specific places rather than in route between them (which btw is perfectly fine: any abstraction is, inevitably and usefully, a simplification of reality, and if for your application's purposes you can abstract away the "traveling between places" state, by all means go for it).

The same applies even more strongly to the variables about required and enough gas -- not natural parts of the object's state So make those local variables, arguments, and return values, for the appropriate methods, i.e. , change the traveling method to: public void travelToDestination(String destination) { int requiredGas = getGasRequiredForDestination(destination); bool enoughGas = hasEnoughGas(requiredGas); if (enoughGas) { totalGas -= requiredGas; location = destination; } } So some of the values needed for computation (precisely those that are part of the object's state) are in instance variables, other (the intermediate results of computations that are not actually part of object state) are local variables, arguments, return values This mixed approach is sounder and more promising than either your original one (with all those "getter" method calls, eep! -) or the one at the other extreme (where everything and its cousin was an instance variable, for mere computational convenience and quite apart from good modeling approaches) Since instance variables and local variables therefore get mixed in most computations, many programming styles require them to have distinguishable names (some languages such as Python and Ruby make that mandatory -- the instance variable location for example would be spelled location or self.

Location but I'm talking about styles for languages that do not force the issue, but still allow you to name that instance variable location with a trailing underscore, or m_location with an m prefix, and the like).

Try reasoning a bit more abstractly: in as much as an instance of your class is modeling a real-world entity (a good thing, when you can do that conveniently), instance variables should be all that you need to represent the state of that thing -- not artefacts such as temporary results of computations, which don't correspond to any real-world "state". So, for example, consider your class: Class Car { String location = "home"; String destination; int totalGas = 0; int requiredGas = 0; boolean enoughGas = false; and critique it based on the test "is this instance variable actually part of the state, or not? ".

By this criterion, location and totalGas seem fine -- a real-world car does indeed have a location, and some amount of gas in it, as part of its real-world state. The others are more dubious. Destination would be fine if you were representing the car at various spots during a trip, or a leg of the trip -- at any given time there would be a present spot, and a destination towards which the car is traveling.

But judging from your code that's not what you're doing: the destination instantly becomes the location if gas is sufficient, so you're using a simplified model of reality where the car is only represented as being in specific places rather than in route between them (which btw is perfectly fine: any abstraction is, inevitably and usefully, a simplification of reality, and if for your application's purposes you can abstract away the "traveling between places" state, by all means go for it). The same applies even more strongly to the variables about required and enough gas -- not natural parts of the object's state. So make those local variables, arguments, and return values, for the appropriate methods, i.e.

, change the traveling method to: public void travelToDestination(String destination) { int requiredGas = getGasRequiredForDestination(destination); bool enoughGas = hasEnoughGas(requiredGas); if (enoughGas) { totalGas -= requiredGas; location = destination; } } So, some of the values needed for computation (precisely those that are part of the object's state) are in instance variables, other (the intermediate results of computations that are not actually part of object state) are local variables, arguments, return values. This mixed approach is sounder and more promising than either your original one (with all those "getter" method calls, eep! -) or the one at the other extreme (where everything and its cousin was an instance variable, for mere computational convenience and quite apart from good modeling approaches).

Since instance variables and local variables therefore get mixed in most computations, many programming styles require them to have distinguishable names (some languages such as Python and Ruby make that mandatory -- the instance variable location for example would be spelled @location or self. Location -- but I'm talking about styles for languages that do not force the issue, but still allow you to name that instance variable location_ with a trailing underscore, or m_location with an m_ prefix, and the like).

Yeah that is a good answer. I need to think about it some more, because there are situations where, if you were needing to extract values from a string and you needed to pull 2 at one time (like with StringTokenizer in java) then you really need to pull them in order to avoid possible problems of some other function in the future accidentially stealing the value you wanted. So since you can't return 2 values this it would at least seem to be better to store it in the object.(You could use a hashmap I guess)... Not sure on that.

– John Baker Jan 2 '10 at 19:28 StringTokenizer is obviously a class that's explicitly not modeling a real-world entity -- that's fine, you absolutely need some of those, too -- and it will have its own notions of "state", which are connected to the progression of its computation rather than to anything "in the real world". It is fine -- just don't mix the two kinds of considerations about "state" inside the same class. – Alex Martelli Jan 2 '10 at 23:02.

Well this question can be explained in detail by wiser heads than me. But here is my take: I tend to try to define classes as some data, and set of operations that need to be performed on them (following class inheritance hierarchy of course). So your approach of encapsulating operations on Car class is right because in this case you would just do Car.

TravelToDestination() and it would be fine. MyMethod { doThis() doThat() checkThis() checkTheOtherThing() } is not wrong in the sense that all your methods above are doing ONE logical operation each (I strongly recommend making methods do ONLY one logical operation) and they are being used correctly. Regarding class data being passed around, it is generally considered a good design if you can encapsulate data and operations on it internally in one class, which seems to be the how you want to do in your example.

Regarding book recommendation, I found for myself Code Complete 2 has an excellent chapter on class design (chapter 6 called Working Classes, I think) that should address doubts like this. I find myself referring to it quite a bit. Either way I think that book should be required reading for all programmers.

I read the first edition of code complete, and Refactoring among others, but it seems that if you don't really work at using it the knowledge leaves pretty quick. I'll definitely check out the new edition though – John Baker Jan 2 '10 at 18:43 I find myself regularly using it for class design etc, but I read it through once a year or two. Do that enough times and a lot of best practices, if you use them, become instinctive.

Trust yourself on this and try it. It works :) – omermuhammed Jan 2 '10 at 19:00.

I think mabe your problem is the idea that there should be "one class where it should go". I don't pretend to understand the domain you are talking about, but some obvious classes relevant to the problem seem to be: car map trip planner location waypoint I don't normally ask a car if it can get from one location to another - I might ask it how far can it go on it's current gas tank load. I wouldn't expect it to know how to get to SF or the mileage, I get some of that from the map, using waypoints.

And all of this should be being coordinated by the trip planner, which comes up with the final answer. All of these classes will have their own specialised member data and will interact using specific member functions. Some of these functions will take instances of other classes as parameters.

The car thing is just a random example I thought up. Just to illustrate the point. I do apologize if it didn't make sense due to that fact.

Normally I wouldn't have it do those functions either but I needed to think of something semi relevant at least – John Baker Jan 2 '10 at 19:05.

Indeed you shouldn't use instance variables to pass data to methods. Why not use function results within one method? Public boolean travelToDestination(string destination) { int requiredGas = getGasRequiredForDestination(destination); boolean enoughGas = hasEnoughGas(requiredGas); if (enoughGas) { location = destination; totalgas -= requiredgas; } } This makes it clear to the reader of your code how getGasRequiredForDestination() and hasEnoughGas() work, without having to look for side effects.It's called principle of least astonishment.

Instance variables should be used to store the state of an object after the control flow has left the class methods, in this case location and totalgas after control returned from travelToDestination(). If the control should return earlier, e.g. When the car is traveling to its destination over multiple steps, then you need to store additional information in field variables.

This is how I would do it: Class Car { String location = "home"; int totalGas = 0; public Car (int totalGas) { this. TotalGas = totalGas; } public boolean travelToDestination(String destination) { int requiredGas = getGasRequiredForDestination(destination); if(totalGas >= requiredGas){ location = destination; totalGas -= requiredGas; return true; } else { return false; } } } The idea is to have as little state as possible, and by "state" I mean variables outside the scope of the method (e.g. Member variables, globals, etc.). I'm not sure if this principle has a specific name, but I've definitely run into similar principles in multiple places while reading.

I can also attest to benefits of reducing state from personal experience. Every extra member variable decreases maintainability and increases the opportunity for bugs to be introduced. Only make member variables if they can't be local variables.

Having state is a core aspect of an object; identity, state, behaviour. It's a pretty poor object that doesn't have internal cohesion. – Chris McCauley Jan 13 '10 at 21:27.

I would pull out a Trip object that contains the origin and destination, and can calculate the distance. I don't think it's the responsibility of a car to understand and operate on the details of a Trip. It is the responsibility of a car to know its mileage and how far it can travel, and to go places.

There's two directions you could go from here. One would be to give the responsibility of performing the trip to the Car, something like: Trip trip = new Trip(origin, destination); Car car = new Car(); car.fillTank(); if (car. CanTakeTrip(trip)) car.

TakeTrip(trip); But this bugs me. It couples Car to Trip and results in two high-level calls to car, both of which operate on a Trip. This suggests you might want to have a Trip member in car and call car.

SetTrip(trip)...but now we know we're down a wrong path. This way leads to madness. Car is now not only coupled to Trip, but can own an object that has nothing to do with Car.

You wouldn't ask "what Trip does that car have? " You'd ask "where is that car going? ".

A car should do simple car things. Fill up with gas. Drive here.

Drive there. It shouldn't know about such things as "Trips. " This suggests that we invert the responsibility and give the job of taking the trip to the Trip itself.

Maybe something like: trip. SetCar(car); trip.travel(); System.out. Println(trip.

Result); // might be "Not enough gas" or "Arrived! Wish you were here! " And inside the call to trip.travel() we might see: public void travel() { if (this.car.

CanTravel(this. Distance)) { car. Travel(this.

Destination); } else { this. Result = "Not enough gas"; } } But we've still coupled Trip to Car, so we might like to pull up a Transport interface that hides the implementation. It makes more sense to me that a Trip would own a Transport than having Car own a Trip.

A Transport interface might be: interface Transport { public boolean canTravel(int distanceInMiles); public void travel(String destination); // seems like we need more than a String here, but you get the idea } Now Car would implement like class Car implements Transport { // implement canTravel and travel } And finally Trip might do something like: class Trip { public void travel(Transport transport) { if (transport. CanTravel(this. Distance)) { transport.Travel(this.

Destination); } else { this. Result = "Not enough gas"; } } } Car car = new Car(); car.fillUp(); Trip trip = new Trip(); // maybe set Transport on constructor? Trip.Travel(car); Or something like this.

I generally try to have my objects to the simplest things that come natural to those objects. If they start obtaining knowledge that doesn't seem to flow naturally into a few (preferably one) responsibility, then I look for the missing Class that I need to define and start trying to invert responsibilities.

An object should have three aspects; state, behaviour and identity. A key metric for an object is that it have high internal cohesion (and low external coupling). There's absolutely nothing wrong with having your methods act on shared internal state.

I know you said that your example was made up to illustrate a point but the 'Car' looks more like a 'Journey' or 'Route.

I cant really gove you an answer,but what I can give you is a way to a solution, that is you have to find the anglde that you relate to or peaks your interest. A good paper is one that people get drawn into because it reaches them ln some way.As for me WW11 to me, I think of the holocaust and the effect it had on the survivors, their families and those who stood by and did nothing until it was too late.