I have two class Task and Award which extends Task. I create this class objects according JSON data. At the moment I have one method that read data passed data and create Task. If I need Award I use method which convert Task to Award. I would like to know if I am following best practice when I want have one method to read data and create object from this data.I know that convertTaskToAward could be moved to class Award.I see three possible path here:Create another method public Award createAward(JsonData json) and use it to create Award instead of converting Task to Award.Replace public Task createTask(JsonData json) with public Award createAward(JsonData json) and then downcast Award to Task if needed.Leave as it is right now but move public Award convertTaskToAward(Task taskForConvert) to class Award.class Taskpublic class Task {    protected int id;    protected String description;    private Type type;    private String difficulty;    protected int repeatCount;    private int points;    protected int tempPoints;    private int money;    private int tempMoney;    protected int time;    protected int tempTime;    private ArrayList<String> accessWords;    private ArrayList<Integer> lettersCount;    protected TaskCallback delegate;    protected boolean isDone;    private BaseGameActivity activity;    private boolean obstacleRemoved;    protected boolean powerupUsed;    private int award;    private SharedPreferences pref;    protected String startTime;    private Date endTimeData;    protected Date startTimeData;    private PauseableTimerHandler timer;    private boolean isBlock;    private boolean isCurrent;    public Task(Type type, int id, String difficulty, int repeatCount, int points, int money, int time, ArrayList<String> accessWords, String description, int award, String startTime, boolean isDone, ArrayList<Integer> lettersCount, boolean isCurrent, BaseGameActivity activity, SharedPreferences pref, TaskCallback delegate) {        this.type = type;        this.id = id;        this.difficulty = difficulty;        this.repeatCount = repeatCount;        this.points = points;        this.money = money;        this.time = time;        this.accessWords = accessWords;        this.startTime = startTime;        this.lettersCount = lettersCount;        this.isDone = isDone;        this.pref = pref;        this.delegate = delegate;        this.description = description;        this.activity = activity;        tempPoints = points;        tempMoney = money;        tempTime = time;        this.award = award;        this.isCurrent = isCurrent;    }    public String getStartTime() {        return startTime;    }    public String getDifficulty() {        return difficulty;    }    public int getId() {        return id;    }    public int getAward() {        return award;    }    public int getTempPoints() {        return tempPoints;    }    public String getDescription() {        return description;    }    public boolean isDone() {        return isDone;    }    public Type getType() {        return type;    }    public int getRepeatCount() {        return repeatCount;    }    public int getPoints() {        return points;    }    public int getMoney() {        return money;    }    public int getTime() {        return time;    }    public ArrayList<String> getAccessWords(Boolean forSave) {        if (!forSave){            return accessWords;        } else {            StringBuilder stringAW = new StringBuilder();            for (String s:accessWords){                stringAW.append(s).append(|);            }            return new ArrayList<String>(Arrays.asList(stringAW.toString()));        }    }    public ArrayList<Integer> getLettersCount(boolean forSave) {        if (!forSave){            return lettersCount;        } else {            StringBuilder stringLetterCount = new StringBuilder();            for (Integer i:lettersCount){                stringLetterCount.append(i);            }            int tempInt = 0;            if (stringLetterCount.toString() != ){                tempInt =  Integer.parseInt(stringLetterCount.toString());            }            return new ArrayList<Integer>(Arrays.asList(tempInt));        }    }    public void reset() {        tempPoints = points;        tempMoney = money;        tempTime = time;        isDone = false;        isBlock = false;    }    public void decreasePoints(int pPoints) {        if (accessWords.contains(CONTINUOUS)) {            points -= pPoints;            if (points <= 0) {                decreaseRepeat();            }        } else if (difficulty.equals(SPECIAL)) {            tempPoints -= pPoints;            if (tempPoints <= 0 && type != Type.Mazes && !isBlock) {                decreaseRepeat();                isBlock = true;            }        } else if (type == Type.Mazes) {            tempPoints -= pPoints;            if (tempPoints > 0) {                decreaseRepeat();                isBlock = true;            }        } else {            tempPoints -= pPoints;            if (tempPoints <= 0) {                decreaseRepeat();            }        }    }    public void decreaseMoney(int pMoney) {        if (!accessWords.contains(SPEND)) {            tempMoney -= pMoney;            if (tempMoney <= 0) {                decreaseRepeat();            }        } else {            money -= pMoney;            if (money <= 0) {                decreaseRepeat();            }        }    }    public void mazeTaskDoneWith(int mistakes) {        if (mistakes <= points) {            decreaseRepeat();        }    }    public void decreaseRepeat() {        if (isDone) return;        if (type != Type.Mazes) {            repeatCount--;            if (repeatCount <= 0) {                delegate.onDone(this);            }        } else {            repeatCount--;            if ((repeatCount <= 0 && !accessWords.contains(SOLVE) || (repeatCount <= 0 && accessWords.contains(SOLVE)))) {                delegate.onDone(this);            }        }    }    public void decreaseeTime() {        if (accessWords.contains(CONTINUOUS)) {            time--;            if (time <= 0 && !isDone) {                decreaseRepeat();            }        } else {            tempTime--;            Log.i(Award id:+id,[Temp]Second pass. +tempTime+ to go.);            if (tempTime <= 0 && !isDone) {                decreaseRepeat();            }        }    }    public Task clone() throws CloneNotSupportedException {        return new Task(this.type, id, difficulty, this.repeatCount, this.points, this.Money, this.time, this.accessWords, this.description, this.award, this.startTime, this.isDone, this.lettersCount, this.isCurrent, this.activity, this.pref, this.delegate);    }    public void done() {        if(!isDone) {            Log.i(Task, id + |  + this.getClass() +  is done:  + description);            isDone = true;        }    }    public TaskCallback getDelegate() {        return delegate;    }}class Awardpublic class Award extends Task {    private Integer oneShot;    private String title;    private Integer tempRepeatCount;    private HashMap<String, Integer> wordsUsedInGame;    private int tempOneShot;    public Award(Type type, int id, int repeatCount, int points, int szyflings, int time, ArrayList<String> accessWords, String description, int award, String startTime, boolean isDone, ArrayList<Integer> lettersCount, BaseGameActivity activity, SharedPreferences pref, TaskCallback pDelegate) {        super(type, id, , repeatCount, points, szyflings, time, accessWords, description, award, startTime, isDone, lettersCount, false, activity, pref, pDelegate);        tempRepeatCount = repeatCount;        if (id == 7) wordsUsedInGame = new HashMap<String, Integer>();    }    public String getTitle() {        return title;    }    public void setTitle(String title) {        this.title = title;    }    public Integer getOneTime() {        return oneShot;    }    public void setOneShot(Integer oneShot) {        this.oneShot = oneShot;    }    @Override    public void decreaseRepeat() {        if (getAccessWords(false).contains(LETTER_GAME) || getAccessWords(false).contains(MAZE_GAME)) {            tempRepeatCount--;            if (tempRepeatCount <= 0) {                isDone = true;                delegate.onDone(this);            }        } else {            repeatCount--;            if (repeatCount <= 0) {                isDone = true;                delegate.onDone(this);            }        }    }    @Override    public void reset() {        tempRepeatCount = repeatCount;        switch (id) {            case 7:                wordsUsedInGame.clear();                break;            case 36:                tempTime = time;                break;            default:                super.reset();                break;        }    }    public void snapDate() {        DateFormat dateFormat = new SimpleDateFormat(HH:mm dd/MM/yyyy);        startTimeData = new Date();        startTime = dateFormat.format(startTimeData);    }    public void checkDate() {        if (!startTime.equals()) {            try {                DateFormat dateFormat = new SimpleDateFormat(HH:mm dd/MM/yyyy);                Date currentDate = new Date();                startTimeData = dateFormat.parse(startTime);                float timePassed = currentDate.getTime() - startTimeData.getTime();                timePassed = ((timePassed / 1000) / 3600);                Log.i(Award id: + id, timePassed);                if (timePassed <= 1) decreaseRepeat();            } catch (ParseException e) {                e.printStackTrace();            }        }    }    @Override    public void decreasePoints(int pPoints) {        if (isDone) return;        switch (id) {            case 10:                tempPoints -= pPoints;                if (tempTime > 0 && tempPoints <= 0) {                    decreaseRepeat();                }                break;            default:                super.decreasePoints(pPoints);                break;        }    }    @Override    public void decreaseeTime() {        if (isDone) return;        switch (id) {            case 10:                if (tempTime > 0) tempTime--;                break;            default:                super.decreaseeTime();                break;        }    }}Main classclass MainActivity {        public void loadTaskAndAward(){                Task task = createTask(jsonDataForTask);                Award award = convertTaskToAward(createTask(jsonDataForAward));        }        public Task createTask(JsonData json){                //..read description,time,money from json                return new Task(dscription,time,money);        }        public Award convertTaskToAward(Task taskForConvert){                Award awardForReturn = new Award(title,taskForConvert.getDescription(),taskForConvert.getTime(),taskForConvert.getMoney());        }}
Best practices to create extended class
java;classes
A Task and an Award seem like very different things. Part of the reason you are having trouble converting them is because they don't want to be the same thing. It is much more likely that a certain Task might provide a certain Award when the player completes the required action. Also, what happens when a Task might produce two different Awards? Do you then need to create another subclass of Task that does the same things for the new Award? It would be better to just give a Task the award it will return as an argument in the constructor.In general, composition is often easier to work with than inheritance. Even in some cases with a clear is-a relationship exists.EDIT based on your response:It sounds like a Task and an Award might share a number of common methods, but are not intrinsically the same type of thing. This is a perfect example for an interface. This would allow you to write a UI that work generically with instances of this interface. From your vague description it might look like:interface ICompletableThing {  boolean isCompleted();  String getTitle();  IStyle getDisplayStyle();}It is possible that you might end up with a base implementation of ICompletableThing that both Task and Award inherit from that implements some shared code, but it does not appear that an Award is a type of Task.To more directly answer your question. I think the best implementation would be to have one function that takes in json and produces an Award and one function that takes in json and produces a Task. Depending on how you are getting your json, you might then have function that returns something like Either<Task, Award> (where you can get the resultant instance in a type-safe way).Instance variables:There are 26 instance variables. That is too many. It seems like you are using Task as just a large bag of state without much care for the organization. Just because the json you parse the data from is organized this way doesn't mean that your object model has to be the same.Some of your variables are private and some of them are protected. It's not clear what decides what is accessible to a child class. For example, startTime is protected yet endTimeData is private. To make matters worse, the variables are not grouped by accessibility, so a reader has to be careful to note which fields are which. (There is a grouping, but it is not clear why these variables are different from the other ones.)There are a number of tempXXX variables. A temp prefix generally means that the variable is temporary and not maintained state. Could these variables have a more descriptive name? Or maybe they belong in a different class to indicate that they are the values that can change during the Task lifetime.Any of the instance variables you don't intend to change should be marked as final. This will prevent you from accidentally changing a value that should be constant. This is even more important when you have some variables that change and some that don't.Decide if you are going to use this. or not and be consistent. In the constructor you leave off this. for the variables where the names are not being shadowed. However, in decreasePoints() the argument is named pPoints. Since there is no documentation to say what pPoints is, I can only assume that the extra p is there so it doesn't shadow the instance variable. My preference is to always prefix an instance variable with an underscore. This way you never have to worry about variable shadowing and it is fewer characters than this..Separation of concerns:public ArrayList<String> getAccessWords(Boolean forSave);Task should not care about what other code does with it's data. Have a class that is in charge of serializing your data and do not put it in your data class. Now everyone who wants to retrieve the access words has to pass an additional argument because some of the serialization formating is managed by the data class. The fact that insert a single String into a list should be an indication that this function is being misused.Additionally, since you don't have a single method that produces the serialized format, some where else, you still need to write a method that pieces together each of these chunks for data.Another reason to not put serialization code in your data object is because you might end op with multiple different formats. In fact, you already do! You have this format you use for saving the state and the json format you talk about parsing from. What happens if you also want to write out the json format?Use interfaces:It is very unlikely that anyone calling your getter cares that the result is actually an ArrayList, List would be sufficient. Since you declared that your method returns a specific List implementation, you had to create a new list that just copies one you already have.return new ArrayList<String>(Arrays.asList(stringAW.toString()));could just bereturn Arrays.asList(stringAW.toString());if your return type was not so restrictive.Hard coded constant strings:You should remove all of them. At the very least, you should replace them with defined constants. This will allow you to change the actual representation (the string value) everywhere it is used with a single edit. It will also remove the chance for subtle errors if you have a typo in on of the hard coded values.However, it is much more likely that you don't want to be using String in the first place. It seems like you have a finite set of values you expect to be an access word. Yet you are using a type that can store infinitely many value. It would make more sense to use an enum to define all of the valid access words. Now it will not be possible to put an invalid String into your list of access words.There are a number of cases where you check if the type is Maze and do a specific thing if this is or isn't the case. You are doing a similar thing with your access words. This kind of programing won't scale well. Every time your get a new type of access word, you have to edit your existing Task method to account for this new type of logic. It would be better have your Task be passed a model that tells it what to do when decreasePoints() is called. That way, when another Type comes along, you just have to define a model for the new type and it will be much less likely that your changes will break what happens when a decreasePoints() is called for a Maze. (This is another example of composition.)Some times you have extra blank lines at the beginning and end of your if blocks. These are not doing anything and can be removed.