2
\$\begingroup\$

My question is mostly directed towards the principles of object-oriented programming.

I have an Android application and one of the activities has a ListView.

In this ListView, I am displaying a list of players and their leagues. Therefore I have constructed the ListViewModel to be either a league or a player, inflating different layouts for the row in each case.

This is the model:

public class PlayersModel {
    private long recordId;

    public String getTitle() { return toString(); }
    public String getPath() { return ""; }
    public long getRecordId() { return recordId; }
    public String toString() { return ""; }

    public boolean isLeague() {
        return (this instanceof EntityLeague);
    }

    public PlayersModel(long id) {
        recordId = id;
    }
}

I am omitting some of the code for simplicity.

This is the class for the players:

public class EntityPlayer extends PlayersModel {    
    private String _lName, _fName, _dob, _path;
    private long _leagueId;

    public String getLastName() { return _lName; }
    public String getFirstName() { return _fName; }
    public String getDateOfBirth() { return _dob; }
    public String getPath() { return _path; }
    public String toString() { return _lName + ", " + _fName; }    

    public EntityPlayer(String lName, String fName, 
            String dob, String path, long leagueId, long playerId) {
        super(playerId);
        this._lName = lName;
        this._fName = fName;
        this._dob = dob;
        this._path = path;
        this._leagueId = leagueId;
    }
}

and this for the leagues:

public class EntityLeague extends PlayersModel {
    private String leagueName, _path;

    public String toString() { return leagueName; }
    public String getLeague() { return leagueName; }
    public String getPath() { return _path; }

    public EntityLeague(String leagueName, String path, long leagueId) {
        super(leagueId);
        this._path = path;
        this.leagueName = leagueName;
    }
}

So in the Adapter I can use the following:

public class PlayersListAdapter extends ArrayAdapter<PlayersModel> {
    private final ArrayList<PlayersModel> modelsArrayList;

    public PlayersListAdapter(Context context, ArrayList<PlayersModel> mal) {
        super(context, R.layout.target_item, mal);
        this.modelsArrayList = mal;
    }

    @Override
    public View getView(int position, View convertView, ViewGroup parent) {
        LayoutInflater inflater = (LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
        PlayersModel m = modelsArrayList.get(position);
        View rowView;
        if (!m.isLeague()) {
            rowView = inflater.inflate(R.layout.target_item, parent, false);
            ImageView imgView = (ImageView) rowView.findViewById(R.id.item_icon);
            TextView titleView = (TextView) rowView.findViewById(R.id.item_title);
            TextView counterView = (TextView) rowView.findViewById(R.id.item_counter);

            if (m.getPath() == null) {
                imgView.setImageResource(R.drawable.no_contact);
            } else {
                try {
                    Bitmap bm = ImageManager.getThumbnail(getContext(), Uri.parse(m.getPath()));
                    imgView.setImageBitmap(bm);
                } catch (IOException e) {
                    imgView.setImageResource(R.drawable.no_contact);
                }
            }
            titleView.setText(m.getTitle());
            counterView.setText(String.valueOf(m.getRecordId()));

        } else {
            rowView = inflater.inflate(R.layout.group_header_item, parent, false);
            TextView titleView = (TextView) rowView.findViewById(R.id.header);
            titleView.setText(m.getTitle());
        }

        return rowView;
    }
}

However, I feel that conceptually, an entity such as a player or a league has no reason to have a Title. I am not sure this sticks in terms of object-oriented programming.

Perhaps the model should be abstract? Or instead of the other class subsclassing the model, the PlayersModel should have local variables for Leagues/Players.

Any suggestions?

\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

What bothers me is, you are referencing EntityLeague subclass in you superclass code. Superclass should work independantly of subclasses. This is causing tight coupling between those classes because if you remove EntityLeague or rename it, your superclass code has to be changed.

public boolean isLeague() {
    return (this instanceof EntityLeague);
}

Also read about 'Liskov Substitution Principle' - https://stackoverflow.com/questions/56860/what-is-the-liskov-substitution-principle

If it does not make sense to have inheritance, you can use composition or interfaces (if contract is involved).

What I think is if you are just looking for common superclass for League and Player, you need better abstraction.

Hope this helps.

\$\endgroup\$
3
  • \$\begingroup\$ Thank you for your answer. Would composition make sense in my case? A model instance will either be a player or a league, it will never have both. \$\endgroup\$ Jan 20, 2015 at 11:39
  • \$\begingroup\$ @parakmiakos What I think is you can make model Interface and define all the methods that are common to both player and league. So implementation can be different in both player and league but they have to follow contact and implement the methods. \$\endgroup\$
    – Ravindra
    Jan 20, 2015 at 11:44
  • \$\begingroup\$ On Code Review it is standard for people to edit your post if there are items to improve, but if people have something new to add, they will do it as a separate answer. You do not score 'reputation' if this is a community wiki post. I am converting it back to a regular one. \$\endgroup\$
    – rolfl
    Jan 20, 2015 at 11:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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