Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am new to the Parcelable interface that I have implemented in my classes. At first they were Serializable, but now I have changed them to Parcelable.

Have I done this the right way, and am I doing everything correctly?

Here are my snippets of code:


public class City implements Parcelable {
    private String code;
    private String name;

    private List<CityWeather> weathers ;

    public City() {
    public String getName() {
        return name;
    public void setName(String name) {
        this.name = name;
    public String getCode() {
        return code;
    public void setCode(String code) {
        this.code = code;
    public List<CityWeather> getWeathers() {
        if(weathers == null) {
            weathers = new ArrayList<CityWeather>();
        return weathers;
    public void addCityWeather(CityWeather w) {

    public void addCityWeathers(List<CityWeather> w) {

    public City(Parcel in) {

    public void writeToParcel(Parcel dest, int flags) {

    private void readFromParcel(Parcel in) {
        code = in.readString();
        name = in.readString();
        in.readTypedList(weathers, CityWeather.CREATOR);

    public static final Parcelable.Creator<City> CREATOR = new Parcelable.Creator<City>() {
        public City createFromParcel(Parcel in) {
            return new City(in);

        public City[] newArray(int size) {
            return new City[size];

    public int describeContents() {
        // TODO Auto-generated method stub
        return 0;


public class CityWeather implements Parcelable {

    private String date;
    private String weatherDescription;
    private int weatherCode;
    private String currentTemperature;
    private String minTemperature;
    private String maxTemperature;
    private String humadity;
    private String wind;
    private String day;
    private String temperatureUnit;

    public String getTemperatureUnit(){
        return temperatureUnit;
    public void setTemperatureUnit(String temperatureUnit){
        this.temperatureUnit = temperatureUnit;
    public String getDate() {
        return date;
    public void setDate(String date) {
        this.date = date;
    public String getWeatherDescription() {
        return weatherDescription;
    public void setWeatherDescription(String weatherDescription) {
        this.weatherDescription = weatherDescription;
    public int getWeatherCode() {
        return weatherCode;
    public void setWeatherCode(int weatherCode) {
        this.weatherCode = weatherCode;
    public String getCurrentTemperature() {
        return currentTemperature;
    public void setCurrentTemperature(String currentTemperature) {
        this.currentTemperature = currentTemperature;
    public String getMinTemperature() {
        return minTemperature;
    public void setMinTemperature(String minTemperature) {
        this.minTemperature = minTemperature;
    public String getMaxTemperature() {
        return maxTemperature;
    public void setMaxTemperature(String maxTemperature) {
        this.maxTemperature = maxTemperature;
    public String getHumadity() {
        return humadity;
    public void setHumadity(String humadity) {
        this.humadity = humadity;
    public String getWind() {
        return wind;
    public void setWind(String wind) {
        this.wind = wind;
    public String getDay() {
        return day;
    public void setDay(String day) {
        this.day = day;

    protected CityWeather(Parcel in) {
        date = in.readString();
        weatherDescription = in.readString();
        weatherCode = in.readInt();
        currentTemperature = in.readString();
        minTemperature = in.readString();
        maxTemperature = in.readString();
        humadity = in.readString();
        wind = in.readString();
        day = in.readString();
        temperatureUnit = in.readString();

    public CityWeather() {
        // TODO Auto-generated constructor stub
    public int describeContents() {
        return 0;

    public void writeToParcel(Parcel dest, int flags) {

    public static final Parcelable.Creator<CityWeather> CREATOR = new Parcelable.Creator<CityWeather>() {
        public CityWeather createFromParcel(Parcel in) {
            return new CityWeather(in);

        public CityWeather[] newArray(int size) {
            return new CityWeather[size];

Error trace:

02-16 17:30:23.014: E/AndroidRuntime(15818): FATAL EXCEPTION: main 
02-16 17:30:23.014: E/AndroidRuntime(15818): java.lang.RuntimeException: Unable to start activity ComponentInfo{ge.mobility.weather/ge.mobility.weather.activity.MainActivity}: java.lang.NullPointerException 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2355) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2391) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at android.app.ActivityThread.access$600(ActivityThread.java:151) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1335) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at android.os.Handler.dispatchMessage(Handler.java:99) 
02-16 17:30:23.014: E/AndroidRuntime(15818):     at android.os.Looper.loop(Looper.java:155) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at android.app.ActivityThread.main(ActivityThread.java:5520) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at java.lang.reflect.Method.invokeNative(Native Method) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at java.lang.reflect.Method.invoke(Method.java:511) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1029) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:796) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at dalvik.system.NativeStart.main(Native Method) 
02-16 17:30:23.014: E/AndroidRuntime(15818): Caused by: java.lang.NullPointerException 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at android.os.Parcel.readTypedList(Parcel.java:1754) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at ge.mobility.weather.entity.City.readFromParcel(City.java:62) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at ge.mobility.weather.entity.City.<init>(City.java:49) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at ge.mobility.weather.entity.City$1.createFromParcel(City.java:67) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at ge.mobility.weather.entity.City$1.createFromParcel(City.java:1) 
02-16 17:30:23.014: E/AndroidRuntime(15818):    at android.os.Parcel.readParcelable(Parcel.java:2108)
share|improve this question

closed as off-topic by rolfl, kleinfreund, syb0rg, Malachi, Simon André Forsberg Feb 16 at 16:06

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Your question must contain working code for us to review it here. For questions regarding specific problems encountered while coding, try Stack Overflow. After getting your code to work, you may edit this question seeking a review of your working code." – rolfl, kleinfreund, syb0rg, Malachi, Simon André Forsberg
If this question can be reworded to fit the rules in the help center, please edit the question.

no you aren't doing everything correctly, you have an error somewhere. –  Malachi Feb 16 at 15:56
If you are aware that the code is not working properly, you should say so in your question. –  Simon André Forsberg Feb 16 at 16:08

1 Answer 1

up vote 3 down vote accepted

In general, the code looks good. I believe the functionality is implemented correctly, and it should work fine.

There are only 2 issues I can see....

  1. you should remove the IDE-generated //TODO lines. There is no need for the //TODO in City.describeContents(). In the two default constructors, the //TODO should be removed as well. If your IDE complains about the empty constructor, you should replace it with a comment that says something like // Empty constructor to keep a public default constructor. Alternatively you can add the (redundant) call to super() like you have in City.

  2. The bigger issue is your visibility of the Parcel-based constructors. In both City, and CityWeather your Parcel constructor should be private (do you have sub-classes of CityWeather that make the protected visibility important? )

EDIT: Your code is failing to re-constitute the City The reason is that weathers will be null when you load the data, and you cannot call in.readTypedList(weathers, CityWeather.CREATOR); with a null weathers. You should either initialize the weathers to new ArrayList<CityWeather>() when it is initialized, or alternatively use the method in.createTypedArrayList(CityWeather.CREATOR);

share|improve this answer
my real problem is that when I navigate through my app sometime it crashes when I look to logcat it tells me that there is some problem with my parcelable city class –  user3314123 Feb 16 at 13:47
Sounds like there is a problem with your code, which means this whole question is off-topic for CodeReview.... and I guess you know that, or you would have posted the logcat output.... –  rolfl Feb 16 at 13:50
thanks a lot it reaaly did help me :) –  user3314123 Feb 16 at 14:26

Not the answer you're looking for? Browse other questions tagged or ask your own question.