Why is this certain type of code considered to be good programming practice and why some is considered bad?

Question

During common programming constructs I have seen that for eg.

class Showframe
{
   psvm(String args[])
   {
      Artframe artframe = new Artframe(new Drawing());
      //some commands
   }
}

class Artframe extends JFrame
{ 
   Drawing drawing;
   public Artframe(Drawing drawing)
   { this.drawing = drawing; }
   //some commands
}

class Drawing
{
   //some things over here
}

I want to know why do we do that? Why not

Drawing class same as above

in ShowFrame

instead of : Artframe artframe = new Artframe(new Drawing());

we write :

Artframe artframe = new Artframe();

and change the constructor of Artframe class to :

drawing = new Drawing();

I mean why do we not do this?? What is wrong in this and why is it considered to be bad programming practice?

@OliverCharlesworth Thank you for I did not know that term before. I added a tag for it and will try to see what it actually is. — Ishan Srivastava, Jun 09 '17 at 13:35
@OliverCharlesworth that is *exactly* how I answer questions when asked what is dependency injection... :) — Eugene, Jun 09 '17 at 13:39
And then, you end up on [What is dependency Injection](https://stackoverflow.com/questions/130794/what-is-dependency-injection). — AxelH, Jun 09 '17 at 13:44
Nota bene: I removed the tags `java 8` and `java 9` because the question has to with neither (at least not specifically). — Nicolai Parlog, Jun 09 '17 at 14:09

score 1 · Accepted Answer · answered Jun 09 '17 at 13:58

Let's take an isolated look at your Artframe class:

public Artframe(Drawing drawing)
{ this.drawing = drawing; }

and

public Artframe()
{ this.drawing = new Drawing(); }

acctually have different semantics.

The first one creates a new artframe with any existing drawing you'll pass from the client. The second one creates a new artframe which has a new (empty?) drawing.

The second one isn't necessarily (but still probably) bad design, if those semantics are your requirement and into fits the "bigger picture".

In most cases the first one will be the better design because of several reasons (I will be somewhat guessing over your overall architecture):

segregation of duties/single responsibility/high cohesion: the Artframe classes job is to frame a Drawing. That should be it's only job. Deciding what drawing to frame would already be a second job.
re-usability: You couldn't use your frame with an existing drawing.
loose coupling: you would normaly design this with an interface instead. This gives you the possibility to easily use different implementations. The second version is hardwired ("coupled") to a specific implementation.

matoni · Answer 2 · 2017-06-09T14:45:29.837

Suppose you want to create simple class that prints greeting with any name:

public class Greeting {

    private final String name;

    public Greetings(String name) {
        this.name = name;
    }

    public void printHello() {
        System.out.println("Hello " + name + "!");
    }
}

When you pass String s = "World", the result would be "Hello World!". This way, you can change the behaviour of the Greeting class. If Greeting class would look like this:

public class Greeting {
    public void printHello() {
        System.out.println("Hello java!");
    }
}

There would not be any option how to change this greeting.

The same applies for classes Artframe and Drawing. Artframe just displays what you provide. Note, that following code snippets would not compile, since they use non-existing graphics context, methods, ...

interface Drawing {
    void paintTo(Canvas c);
}

class Artframe extends JFrame {
    Drawing drawing;

    public Artframe(Drawing drawing) {
       this.drawing = drawing;
    }

    protected void paint(Canvas c) {
        drawing.paintTo(c);
    }
}

Drawing can be implemented in various ways e. g. one implementation draws to graphics context circle, other draws rectangle, ....

public CircleDrawing implements Drawing {
    Point centerPoint = new Point(0,0);
    int radius = 5;
    int strokeWidth = 1;

    public void paintTo(Canvas c) {
        c.drawCircle(centerPoint, radius, strokeWidth);
    }
}

public RectangleDrawing implements Drawing {
    Point topLeft = new Point(0,0);
    Dimension dim = new Dimension(100, 50);
    int strokeWidth = 1;

    public void paintTo(Canvas c) {
        c.drawRect(topLeft , dim, strokeWidth);
    }
}

To display circle in Artframe you just have to pass CircleDrawing instance:

new Artframe(new CircleDrawing());

If you need to display rectangle, then:

new Artframe(new RectangleDrawing());

Again, if your Artframe class would not have Drawing dependency, i. e. used drawing would be hard-coded into the Artframe, then you would not be able to change it's behaviour - displayed would always be the same Drawing:

class Artframe extends JFrame {
    Drawing drawing = new RectangleDrawing();

    protected void paint(Canvas c) {
        drawing.paintTo(c);
    }
}

Why is this certain type of code considered to be good programming practice and why some is considered bad?

2 Answers2