Don't expose class internals

I'm going to disagree a bit with Robert Martin, the author of Clean Code. In his G14 "Feature Envy" smell he uses the example of an method on an HourlyPayCalculator class that takes an HourlyEmployee argument and then uses its properties to do its job. That makes the method "envy" the HourlyEmployee class - the method "wishes it was inside the HourlyEmployee class".

So far, so good. Unfortunately, Robert continues with a counter-example to the feature envy smell; he uses the following example (Java code):

  public class HourlyEmployeeReport {
    private HourlyEmployee employee;

    public HourlyEmployeeReport(HourlyEmployee e) {
      this.employee = e;
   }

    String reportHours() {
      return String.format("Name: %s\tHours: %d.%1d\n",
      employee.getName(),
      employee.getTenthsWorked()/10,
      employee.getTenthsWorked()%10);
    }
  }

Robert says: "Clearly, the reportHours method envies the HourlyEmployee class. On the other hand, we don't want HourlyEmployee to have to know about the format of the report. Moving that format string into the HourlyEmployee class would violate several principles of object oriented design."

I believe there's a way to solve both problems. The HourlyEmployee class has to know about the concept of reporting; that's why we're exposing the getName() method. The only decision is whether we hide this concept or make it explicit. Here's a way we could make it explicit (I'll use C# because, while I can read Java, I'm not confident enough to write it):

  public interface HourlyEmployeeReporting {
    void Report(string name, double tenthsWorked);
  }

  public class HourlyEmployee {
    private string name;
    private double tenthsWorked;
    //... other stuff, but keep the internals hidden

    public void Report(HourlyEmployeeReporting reporter) {
      reporter.Report(name, tenthsWorked);
    }
  }

  public class HourlyEmployeeReporter: HourlyEmployeeReporting {
    void Report(string name, double tenthsWorked) {
      var report = string.format("Name: {0}\tHours: {1}.{2}\n",
        name, tenthsWorked / 10, tenthsWorked % 10);
      // do something with the report value, like writing it to a Logger
    }
  }

This way we haven't sacrificed any of the object oriented principles: everything is encapsulated, we can still have reporting and, quite importantly, reporting is exposed as a concept instead of being hidden just because we can use the internals of the HourlyEmployee class to do it.

[Note: I am not saying that exposing getName() doesn't work. I am saying that the way I'm showing is more object oriented.]