0

I have a car object. The car cannot be driven unless it is turned on. When should I check to see if the car is on before I try to drive it?

In the main program?

class Program { static void Main(string[] args) { Car myCar = new Car(); if (myCar.isOn == false) //check here if car is on { myCar.Start(); } myCar.Drive(); } } class Car { public bool isOn; public void Drive() { Console.WriteLine("Car is driving"); } public void Start() { Console.WriteLine("Car is starting"); isOn = true; } }

Or in the drive method of the class itself?

class Program { static void Main(string[] args) { Car myCar = new Car(); myCar.Drive(); } } class Car { public bool isOn; public void Drive() { if (this.isOn == false) //Check here if the car is on { this.Start(); } Console.WriteLine("Car is driving"); } public void Start() { Console.WriteLine("Car is starting"); isOn = true; } }

This is an extremely simplified example, but when you have a large complex object with complicated logic, the need for organizing and controlling the object's behavior becomes apparent. Where is the proper place of determining and correcting an object's readiness before calling one of its methods? Should the method itself be responsible or the subroutine calling that method?

Improve this question
2
  • (a) Make your abstractions simple for the calling clients to use -- as in your #2. (b) if there are multiple calling clients it will be more DRY to put such logic inside the encapsulation than to expect each of the calling clients to perform this logic. (c) if you generalize to other vehicle types, it is possible that some won't need "starting". Commented Jan 24, 2019 at 0:04
  • 1
    Please don't say "if (booleanExpression == false)". Commented Jan 24, 2019 at 4:19

2 Answers 2

Reset to default
6

Generally, the methods should check that they are capable of being called (or ready to be called). You want the details of the class to be internal to the class itself.

Two sets of design principles are SOLID and GRASP. In the example you gave, the closest principle is the "information expert" from GRASP. Your vehicle/car class is the expert on knowing whether it is ready to be driven or not.

Improve this answer
2

As my personal preference I would have a public get "isCarOn" but i would check it in the driveCar method also. This way a user could check if it was on AND the interbal methods could also check. (I know it was simplified but the idea still crosses to other classes too)

I think many small simple worker methods make troubleshoting easier, and inter-callability meanns minimal code repetition.

Improve this answer
8
  • An alternative would be to have the call be named "ensureCarOn()". Caller doesn't have to check whether it is already on, just knows that, after that cal, the car is on. (or some Exception gets thrown) Commented Jan 24, 2019 at 2:25
  • 1
    True, the caller may not need to ensure the car is on, but by providing the public method you pre-plan for a use where the caller may need to know if its on. Say if the caller has other functions like lock doors that needs to know because the original car class didn't have power doors Commented Jan 24, 2019 at 2:29
  • Tell don't Ask Caller should just "tell* the car to lock the doors. Not Ask "how many doors do you have?", "Are they power", etc... Commented Jan 24, 2019 at 5:24
  • No, i meant if the caller was an extension of the car class and had a lock doors function it might call isCarOn to decide if it needs to fire its own lockDoors, not ask car about those features, it owns the features and only asks the car if it is on so it can decide for itself what to do. Commented Jan 24, 2019 at 5:29
  • Ok. But then isCarOn() should be protected, not public. As soon as you add it to the public API you are pretty much stuck with it. Many would try to severely limit protected methods as well, though I am not on of them. Commented Jan 24, 2019 at 5:39

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.