Advice writing Losely Coupled code with Agile methods or otherwise

Question

I have been reading Robert C. Martin's (aka Uncle Bob) books very intensely as of late. I have found a lot of the things he talks about to be real life savers for me (small function size, very descriptive names for things, etc).

The one problem I haven't gotten past yet is code coupling. One problem I keep having over and over is that I will create a object, like something that wraps an array for instance. I do will some work on it in one class, but then have to call another class to do work on it in a different class. To the point where I'm passing the same data 3-4 levels deep, and that just doesn't seem to make sense because it's hard to keep tract of all the places this object is getting passed, so when it's time to change it I have a ton of dependencies. This seems like anything but a good practice.

I was wondering if anyone knew of a better way to deal with this, it seems that Bob's advice (though it's probably me misunderstanding it) seems to make it worse because it has me creating lots more classes. Thanks ahead of time.

EDIT: By Request a real world example (yes I totally agree that it is hard to make sense of otherwise):

class ChartProgram () {
int lastItems [];

  void main () {
  lastItems = getLast10ItemsSold();
  Chart myChart = new Chart(lastItems);
  }
}
class Chart () {
  int lastItems[];
  Chart(int lastItems[]) {
  this.lastItems = lastItems;
  ChartCalulations cc = new ChartCalculations(this.lastItems);
  this.lastItems = cc.getAverage();
}
class ChartCalculations {
  int lastItems[];
  ChartCalculations (int lastItems[]){
  this.lastItems = lastItems;
  // Okay so at this point I have had to forward this value 3 times and this is 
  // a simple example. It just seems to make the code very brittle
  }
  getAverage() {
  // do stuff here
  }

}

Answer 1

Code Complete calls it "Streamline parameter passing":

If you're passing a parameter among several routines, that might indicate a need to factor those routines into a class that share the parameter as object data.
Streamlining parameter is not a goal, per se, but passing lots of data around suggests that a different class organization might work better.

Answer 2

It seems to me you are misunderstanding coupling with (relatively) complex software development.

If you are passing the data around to be consumed, be it via a class or more than one, all the consumers can do is to access the public interface of the class with the data. This is fine regarding to coupling, because you can change how are those classes implemented internally without breaking anything as long as you have your interface constant. It is expected that data could have many clients, as long as they don't depend on anything but the specified public interface.

You would have a problem with coupling if you accessed private members of the classes, or if you passed a reference of the array another class exposes to be modified by third parties, and so on.

It's true though that if you want to change that public interface and the class is being consumed in many places but that would happen anyway, even if you used composition instead of parameter passing, so the most important thing is to design your public interfaces well enough so changes are not a common occurrence.

Summing up, while sometimes this may point to a design problem (where a better design might translate into a class hierarchy where you don't need to pass that much data around), it isn't per se something bad, and in certain cases it's even needed.

EDIT: First of all, is CartCalculations really needed or are you creating that class just to follow some rule? Then, if CartCalculations is warranted, why are you passing an int[] instead of CartItems where you can have control about what is done and how to the list of items? Finally, why do you feel that is brittle? Because you might forget to pass the parameter (compile error, no biggie)? Because somebody might modify the list where it shouldn't (somewhat controllable via having the CartItems which would be the only one loading the data)? Because if you need to change how items are represented (again no biggie if you wrap the array in a class where you could make that kind of changes).

So, assuming all this hierarchy is warranted, and changing Chart to Cart because it makes more sense to me:

class CartProgram () {
  CartItems lastItems;

  void main () {
    lastItems = getLast10ItemsSold();
    Cart myChart = new Cart(lastItems);
  }
}
class Cart () {
  CartItems lastItems;
  Cart(CartItems lastItems) {
  this.lastItems = lastItems;
  CartCalulations cc = new CartCalculations(this.lastItems);
  cc.getAverage();
}

class CartCalculations {
  CartItems lastItems;
  CartCalculations (CartItems lastItems){
  this.lastItems = lastItems;
  // Okay so at this point I have had to forward this value 3 times and this is 
  // a simple example. It just seems to make the code very brittle
  }
  getAverage() {
  // do stuff here
  }
}

class CartItems {
   private List<Item> itemList;

   public static CartItems Load(int cartId) {
       //return a new CartItems loaded from the backend for the cart id cartId
   }

   public void Add(Item item) {
   }

   public void Add(int item) {
   }

   public int SumPrices() {
       int sum = 0;
       foreach (Item i in itemList) {
          sum += i.getPrice()
       }
       return sum;
   } 
}

class Item 
{
    private int price;
    public int getPrice() { return price; }
}

For a well architected chart library, see http://www.aditus.nu/jpgraph/jpgarchitecture.php

Answer 3

To avoid coupling, in your example, I would create some kind of DataProvider that can fetch the data points to your graph. This way you can have several different types of datasources that you change depending on what you want kind of data you want to chart. E.g.

interface DataProvider
{
  double[] getDataPoints(); 
}

class Chart 
{
 void setDataProvider(DataProvider provider)
 {
   this.provider = provider;
 }
}

And then I would skip the ChartCalculation class completely and just do the calculations in the chart class. If you feel the need to reuse the code for calculating stuff I would create a toolkit class(similar to the Math class in Java) that can calculate average, median, sums or whatever you need. This class would be general and it would know nothing about the chart or what last items means.

Answer 4

Looking at your sample code, your Chart class is tightly coupled to the ChartCalculation class (it's instantiating a new instance of it). If the Chart class uses the ChartCalculation through an interface (IChartCalculation in my sample code) you can remove this coupling. The concrete implementation of IChartCalculation used by the Chart class could be passed to the Chart class by it's constructor (dependency injection), perhaps by a factory class. An alternative might be to use an IOC framework (though often this may introduce more complexity than necessary). In this way the Chart class can use different concrete implementations of IChartCalculation and both can vary independently as long as they are coded to/implement the interface.

class ChartProgram () 
{
    int lastItems [];

    void main () {
    lastItems = getLast10ItemsSold();
    Chart myChart = new Chart(lastItems, new ChartCalculations());
}

class Chart
{
    int[] lastItems;

    IChartCalculation chartCalculations;

    public Chart(int[] lastItems, IChartCalculations cc)
    {
        this.lastItems = lastItems;
        chartCalculations = cc;
        int average = cc.GetAverage(lastItems);
    }
}

interface IChartCalculation
{
    int GetAverage(int[] lastItems);
}


class ChartCalculation : IChartCalculation
{
    public int GetAverage(int[] lastItems)
    {
        // do stuff here
    }
}

Answer 5

Some coupling is useful in an application, and I would say necessary, and the fact that you will get compiler errors if you change your type doesn't necessarily make it brittle.

I think how to look at this is based on how you will be using the data, and how types may change.

For example, if you expect that you will not always use an int[], but later will need ComplexDataType[] then you may want to put this data into it's own class, and pass that class around as a parameter. That way if you need to make changes, you want to extend it from just mapping integer data to complex numbers, then you would create new setters and getters, so that you insulate the data from the application, and you are free to change the implementation all you want with little care, as long as you keep to the contracts you already agreed to. So, if you state that int arrays can be used in the constructor, or as a setter, then allow that, but also allow other types, ie double, ComplexDataType for example.

One thing to look at is if you want the transformations that a class may do to affect all the instances of your data. So, you have your data transfer object, and you have two threads. One will create a 3D spherical graph, and the other a 2D pie chart. I may want to modify the DTO I have in each graph, to get it into a format for graphing, but it would cause all kinds of problems for the pie chart if the data is changed for spherical coordinates. So, at that point, as you pass data around, you may want to take advantage of the final keyword on the parameter, and make your DTO immutable, so anything you pass out will be a copy, but allow them to replace the information if explicitly requested (they call a setter).

I think this will help to reduce problems as you call several classes deep, but you may need to return the DTO if it was modified, but in the end it is probably a safer design.