tags:

views:

183

answers:

10

I am trying to learn object oriented programming in the context of Java. I am writing a fairly simple code, but am getting this error:

Exception in thread "main" java.lang.NullPointerException
at Advisor_score.Rating.Score(Rating.java:12)
at Advisor_score.Rating.main(Rating.java:25)

Here is my code:

package Advisor_score;
public class Rating {
    double [] Ratings;
    double sum;
    double raw_advisor;
    double advisor_score;
    public Rating (double [] x){
        double [] Ratings = x;
    }

    public double Score(){
        for(int i=2;i<Ratings.length;i++){
            sum+=Ratings[i];
        }
        raw_advisor=((sum-(3*(Ratings.length-2)))/4);
        advisor_score= 2.5+(2.5*(1-Math.pow(Math.E, -.5*raw_advisor)));
        return advisor_score;
    }

    public void print(){
        System.out.println(advisor_score);
    }

    public static void main(String[] args){ 
        double p1_1[] = {101,1,1,1.5,.5};
        Rating d = new Rating(p1_1);
        d.Score();
        d.print();
    }
}

I have been looking at this for hours and cannot figure out the problem with the code. I am new to programming can anyone help me? Thanks in advance!

+4  A: 

Because your Ratings which is member of your class is not initialized just declared so it is currently referring to NULL. and you can't execute method on NULL

in constructor you should use this.Ratings = x; which will initilize your member variable which you are trying to use in score method.

I would suggest you to go through tutorials

org.life.java
+3  A: 

The problem, is that you are defing a new variable name Ratings intead of using the one you already have defined, so Rating is not defined when calling score.

In your constructor, you shouldn't redefine it, just use it.

public Rating (double [] x){
    Ratings = x;
        }


I think this should work.

David Brunelle
+11  A: 
public Rating (double [] x){
    double [] Ratings = x;
}

Here you're declaring a local variable Ratings and assigning x to it. Right afterwards, the variable goes out of scope.

What you want to do is Ratings = x;, which would set the instance variable Ratings to x instead of creating a new and unused local variable.

sepp2k
+2  A: 

You got two variables with the same name

Change

 public Rating (double [] x){
      double [] Ratings = x;
 }

to

public Rating (double [] x){
    Ratings = x;
}
djna
+6  A: 

Here is your code, nicely formatted and the bug fixed:

package com.yourdomain.advisor.score;

public class Rating{

    double[] ratings;
    double sum;
    double rawAdvisor;
    double advisorScore;

    public Rating(final double[] x){
        this.ratings = x;
    }

    public double score(){
        for(int i = 2; i < this.ratings.length; i++){
            this.sum += this.ratings[i];
        }
        this.rawAdvisor = (this.sum - 3 * (this.ratings.length - 2)) / 4;
        this.advisorScore =
            2.5 + 2.5 * (1 - Math.pow(Math.E, -.5 * this.rawAdvisor));
        return this.advisorScore;
    }

    public void print(){
        System.out.println(this.advisorScore);
    }

    public static void main(final String[] args){
        final double p1_1[] = { 101, 1, 1, 1.5, .5 };
        final Rating d = new Rating(p1_1);
        d.score();
        d.print();
    }
}

You were assigning a value to an unused local variable instead of a field.

And I added a few changes according to java conventions:

  • package names are lower case letters and periods only and should reflect your domain name
  • class names are in TitleCase
  • method, field and local variable names are in camelCase

See the Sun Java Naming Conventions

Also, do yourself a favor: use an IDE like Eclipse and turn code formatting on

seanizer
+1 The return value of score is still just discarded though when called as d.score() in main. Looks like it should really be void
barrowc
Yeah i know, didn't touch that. One option would be to return Rating and make it a fluent interface.
seanizer
+1  A: 

You have :

double [] Ratings;  

What is Ratings? Needs to be initialized or be set some non null value.

fastcodejava
A: 

The problem arises from trying to reference the Ratings instance variable defined at the top of the class. This is never assigned to because within your constructor you are creating a local variable Ratings and assigning x to that. The local variable is then immediately discarded leaving the Ratings instance variable pointing to null.

The fix is simple:

public Rating (double [] x){
  this.Ratings = x;
}
Adamski
A: 

In your constructor, you are assigning the array to a local variable rather than the Ratings field:

public Rating (double [] x){
    double [] Ratings = x;
}

should be

public Rating (double [] x){
    Ratings = x;
 }

or even, if you want to be more explicit

public Rating (double [] x){
    this.Ratings = x;
}

I would also advise capitalising Ratings in this way, as it is against Java conventions which makes it confusing to read; google for more information on the topic of naming conventions.

PeterT
A: 

Your constructor declares a new variable Ratings that hides the instance variable.

Package names should begin with lower case letters.

Variable names should begin with lower case letters.

Qwerky
A: 

I think in constructor you meant to initialize the class member public double [] Ratings,but in constructor you are creating another double [] Ratings,which is local to constructor.Change the constructor to

public Rating (double [] x){
double [] Ratings = x;
    }

It will work

Anil Vishnoi