1
\$\begingroup\$

I have to calculate a user's location periodically and store & send it over the network on Android. I also have to indicate errors during calculation of location as well. The errors could come from the location API or user having disabled their GPS. I have written the following class for this purpose:

/**
 * Store info about a location and make it available in various formats.
 * 
 * @author deepankar
 */
public class LocationInfo {

    /* Labels for keys when converting to key-value pairs */
    private static final String KEY_STATUS_CODE = "status_code";
    private static final String KEY_STATUS = "status";
    private static final String KEY_PROVIDER = "provider";
    private static final String KEY_ALTITUDE = "altitude";
    private static final String KEY_TRAVELSPEED = "travelspeed";
    private static final String KEY_ACCURACY = "accuracy";
    private static final String KEY_LONGTITUDE = "longitude";
    private static final String KEY_LATITUDE = "latitude";
    private static final String KEY_TIMESTAMP = "timestamp";
    private static final String KEY_ID = "id";

    /**
     * The unique id for this location.
     */
    private int mId;

    /**
     * The time of capture of this location as reported by location API. In
     * milliseconds since January 1, 1970 00:00:00.0 UTC.
     */
    private String mTimeStamp = "0";
    /**
     * The latitude.
     */
    private String mLat = "0.0";
    /**
     * The longitude.
     */
    private String mLong = "0.0";
    /**
     * The accuracy of measurement, in meters.
     */
    private String mAcc = "0.0";
    /**
     * The travelspeed in m/s.
     */
    private String mTravelSpeed = "0.0";
    /**
     * The altitude in meters.
     */
    private String mAltitude = "0.0";
    /**
     * The location provider API name.
     */
    private String mProvider = "fused";
    /**
     * The status of the location info. This is used to indicate errors from API
     * calls or GPS settings. "OK" if the location is valid.
     */
    private String mStatus = "CATFOREST";
    /**
     * Code corresponding to the error indicated by status.
     */
    private String mStatusCode = "-1";
    /**
     * Whether this location was sent to the server.
     */
    private String mSent = "false";

    /**
     * @param id
     *            The unique id for this location.
     * @return The LocationInfo
     */
    public LocationInfo id(int id) {
        this.mId = id;
        return this;
    }

    /**
     * @param time
     *            The time in milliseconds since January 1, 1970 00:00:00.0 UTC.
     * @return The LocationInfo
     */
    public LocationInfo timestamp(long time) {
        this.mTimeStamp = String.valueOf(time);
        return this;
    }

    /**
     * @param lat
     *            The latitude of this location.
     * @return The LocationInfo
     */
    public LocationInfo lat(double lat) {
        this.mLat = String.valueOf(lat);
        return this;
    }

    /**
     * @param lat
     *            The longitude of this location.
     * @return The LocationInfo
     */
    public LocationInfo lng(double lng) {
        this.mLong = String.valueOf(lng);
        return this;
    }

    /**
     * @param acc
     *            The accuracy of measurement, in meters.
     * @return The LocationInfo
     */
    public LocationInfo acc(double acc) {
        this.mAcc = String.valueOf(acc);
        return this;
    }

    /**
     * @param provider
     *            The location provider API name.
     * @return The LocationInfo
     */
    public LocationInfo provider(String provider) {
        this.mProvider = provider;
        return this;
    }

    /**
     * 
     * @param status
     *            The status of the location info. This is used to indicate
     *            errors from API calls or GPS settings. "OK" if the location is
     *            valid.
     * @return The LocationInfo
     */
    public LocationInfo status(String status) {
        this.mStatus = status;
        return this;
    }

    /**
     * 
     * @param statusCode
     *            Code corresponding to the error indicated by status.
     * @return The LocationInfo
     */
    public LocationInfo statusCode(int statusCode) {
        this.mStatusCode = String.valueOf(statusCode);
        return this;
    }

    /**
     * 
     * @param travelSpeed
     *            The travelspeed in m/s.
     * @return The LocationInfo
     */
    public LocationInfo travelSpeed(double travelSpeed) {
        this.mTravelSpeed = String.valueOf(travelSpeed);
        return this;
    }

    /**
     * 
     * @param altitude
     *            The altitude in meters.
     * @return The LocationInfo
     */
    public LocationInfo altitude(double altitude) {
        this.mAltitude = String.valueOf(altitude);
        return this;
    }

    /**
     * Marks this location info as stored in db.
     * 
     * @return The LocationInfo
     */
    public LocationInfo sent() {
        this.mSent = "true";
        return this;
    }

    /**
     * Get this LocationInfo as a ContentValues key-value set.
     * 
     * @return LocationInfo as ContentValues key-value set.
     */

    public ContentValues toDbRow() {
        ContentValues row = new ContentValues();
        row.put(LocInfoTable.COL_TIMESTAMP, mTimeStamp);
        row.put(LocInfoTable.COL_LATITUDE, mLat);
        row.put(LocInfoTable.COL_LONGITUDE, mLong);
        row.put(LocInfoTable.COL_ACCURACY, mAcc);
        row.put(LocInfoTable.COL_PROVIDER, mProvider);
        row.put(LocInfoTable.COL_STATUS, mStatus);
        row.put(LocInfoTable.COL_STATUS_CODE, mStatusCode);
        row.put(LocInfoTable.COL_SENT_TO_CT, mSent);
        return row;
    }

    /**
     * Get this LocationInfo as a JSON Object.
     * 
     * @return LocationInfo as JSON.
     */
    public JSONObject toJSON() {
        JSONObject message = new JSONObject();
        try {
            message.put(KEY_ID, this.mId);
            message.put(KEY_TIMESTAMP, this.mTimeStamp);
            message.put(KEY_LATITUDE, this.mLat);
            message.put(KEY_LONGTITUDE, this.mLong);
            message.put(KEY_ACCURACY, this.mAcc);
            message.put(KEY_PROVIDER, this.mProvider);
            message.put(KEY_STATUS, this.mStatus);
            message.put(KEY_STATUS_CODE, this.mStatusCode);
        } catch (JSONException e) {
            Logger.error("Error while converting location info to JSON");
        }
        return message;
    }

    /**
     * Get this LocationInfo as a Hashtable.
     * 
     * @return LocationInfo as Hashtable.
     */
    public Hashtable<String, String> toHashtable() {
        Hashtable<String, String> table = new Hashtable<String, String>();
        table.put(KEY_ID, String.valueOf(this.mId));
        table.put(KEY_TIMESTAMP, this.mTimeStamp);
        table.put(KEY_LATITUDE, this.mLat);
        table.put(KEY_LONGTITUDE, this.mLong);
        table.put(KEY_ACCURACY, this.mAcc);
        table.put(KEY_TRAVELSPEED, this.mTravelSpeed);
        table.put(KEY_ALTITUDE, this.mAltitude);
        table.put(KEY_PROVIDER, this.mProvider);
        table.put(KEY_STATUS, this.mStatus);
        table.put(KEY_STATUS_CODE, this.mStatusCode);
        return table;
    }

    @Override
    public String toString() {
        Long.valueOf(mTimeStamp);
        return "Lat: " + mLat + ", Long: " + mLong + ", Acc: " + mAcc
                + ", Time: " + mTimeStamp;
    }

}
  • Am I using the correct pattern to store multiple fields for my purpose?
  • What can I improve in my code style?
  • What other improvements do you suggest?
\$\endgroup\$
1
\$\begingroup\$
  1. You are using strings to store numbers and booleans, bad idea™. Store them as the type they need to be and only convert when needed (JSON allows floating point and boolean values without the explicit conversion to string)

  2. I'm not a fan of hungarian notation. The m prefix is unnecessary and just adds clutter to the names. A good ide will allow you to highlight field differently from local variables.

  3. You should add getters for the properties.

  4. You have 2 different key-to-field mappings. I suggest that there should only be one of them.

  5. You can convert to 3 different objects I suggest that there should also be a way to get a LocationInfo object from one of those (and make sure the round-trip is valid).

  6. add equals and hashcode

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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