1

My goal is to fix the bugs in several Java classes using a tool that automatically generates tests to find these bugs. One bug is a NullPointerException that is caused because two attributes are initialized to null and passed into an object. Again, this was automatically generated by the tool so my goal is to fix the class files.

Here is the code that the tool generated:

public void test1() throws Throwable {
        if (debug)
            System.out.format("%n%s%n", "ErrorTest0.test1");
        Point point0 = null;
        Point point1 = null;
        Line line2 = new Line(point0, point1); // Line 16
    }

And here is the constructor that is causing the problem:

        public Line(Point p1, Point p2)
        {
                this.point1 = new Point(p1.x,p1.y); // Line 17
                this.point2 = new Point(p2.x,p2.y);
        }

And the constructor for Point:

        public Point(double x, double y)
        {
                this.x = x;
                this.y = y;
        }

Here is the error message I'm getting from the tool:

java.lang.NullPointerException
        at Line.<init>(Line.java:17)
        at ErrorTest0.test1(ErrorTest0.java:16)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
        at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
        at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
        at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
        at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
        at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
        at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
        at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
        at org.junit.runners.Suite.runChild(Suite.java:128)
        at org.junit.runners.Suite.runChild(Suite.java:27)
        at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
        at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
        at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
        at org.junit.runners.Suite.runChild(Suite.java:128)
        at org.junit.runners.Suite.runChild(Suite.java:27)
        at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
        at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
        at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
        at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
        at org.junit.runner.JUnitCore.run(JUnitCore.java:115)
        at org.junit.runner.JUnitCore.runMain(JUnitCore.java:77)
        at org.junit.runner.JUnitCore.main(JUnitCore.java:36)

I'm guessing I'm supposed to include something in the constructors that doesnt allow this to be null?

AdrianHHH
  • 13,492
  • 16
  • 50
  • 87
  • 3
    `null` means not a valid value; what should a line between two invalid points look like? what about between a valid point and an invalid point? – Elliott Frisch Sep 07 '19 at 15:19
  • 1
    Does it make sense to pass null into the constructor? If this were my code, I'd test for null, and if present, throw my own custom exception, one that I'd handle however best to do – Hovercraft Full Of Eels Sep 07 '19 at 15:19
  • Initialize your `Point` instances, before passing them as parameters to a `Line`-type object – Soutzikevich Sep 07 '19 at 15:31
  • Well, imo it's not a test because there is no assertion. So you need to add some assertions. Regarding null values, **you** have to decide what you want, either throw an exception (which I would do) or suppress it and don't initialize the fields (which I think is a bad design). – Arnaud Claudel Sep 07 '19 at 15:37
  • Also look at this software engineering stack exchange answer: [Best way to handle nulls in Java?](https://softwareengineering.stackexchange.com/questions/51076/best-way-to-handle-nulls-in-java) – Hovercraft Full Of Eels Sep 07 '19 at 16:41
  • Please don't make more work for others by vandalizing your posts. By posting on the Stack Exchange (SE) network, you've granted a non-revocable right, under the [CC BY-SA 4.0 license](//creativecommons.org/licenses/by-sa/4.0), for SE to distribute the content (i.e. regardless of your future choices). By SE policy, the non-vandalized version is distributed. Thus, any vandalism will be reverted. Please see: [How does deleting work? …](//meta.stackexchange.com/q/5221). If permitted to delete, there's a "delete" button below the post, on the left, but it's only in browsers, not the mobile app. – Makyen Sep 22 '19 at 18:38

2 Answers2

2

There are three approaches you could use to update the line method. Hopefully these would be picked up by your test tool.

Require the parameters in the constructor as not null. This is the more modern approach.

public Line(@NotNull Point p1, @NotNull Point p2) {

    this.point1 =  Objects.requireNonNull(p1);
    this.point2 =  Objects.requireNonNull(p2);
}

Throw an exception when the parameters are set to null. Your test case should allow you to expect an exception for that use case.

public Line(Point p1, Point p2) throws IllegalArgumentException {

    if (p1 == null || p2 == null) {
        throw new IllegalArgumentException("point cannot be null");    
    } 

    this.point1 = p1;
    this.point2 = p2;
}

Have a sensible default behavior when null is passed in. What is sensible depends entirely on your use case.

public Line(Point p1, Point p2) {

    if (p1 != null) {
        this.point1 = p1;
    } else {
        this.point1 = new Point(0.0, 0.0);
    }

    if (p2 != null) {  
        this.point2 = p2;
    } else {
        this.point2 = new Point(0.0, 0.0);
    }     
}
CheeseFerret
  • 597
  • 11
  • 21
-2

Is that enought?

public Line(Point p1, Point p2)
{
    if (p1 == null || p2 == null)
        throw new IllegalArgumentException("One or both points are null");

    this.point1 = new Point(p1.x,p1.y);
    this.point2 = new Point(p2.x,p2.y);        
}

You are trying to access field of the class that do not initialize.

Pivoter
  • 360
  • 1
  • 3
  • 10
  • 1
    This really doesn't explain anything or go into much depth, I'm afraid. – Hovercraft Full Of Eels Sep 07 '19 at 15:19
  • What result do you want to see when the points are nulls? Usually, in such cases, throw an `IllegalArgumentException`. – Pivoter Sep 07 '19 at 15:32
  • 2
    Now if one (or both) of the passed points are `null`, you silently create a Line object with undefined state, which will crash some time later. You're just hiding the problem, making debugging harder. – Robert Sep 07 '19 at 15:59
  • As I said above - it all depends on what the author wants. However, I agree that this is not the best solution – Pivoter Sep 07 '19 at 16:04
  • 1
    Then improve the answer to handle these contingencies (or delete this answer). Please see CheeseFerret's answer for an example of this. – Hovercraft Full Of Eels Sep 07 '19 at 16:42