Code Review request for MapPage.java

[GoogleCodeExporter]

What steps will reproduce the problem?
1. We also need one more class that updates gps location every given time. 
I implemented this class inside of MapPage.java at first and it worked 
fine. But when I try to put in separate file then code breaks. Please see 
code for class below.
2. Request for code review

//Separate class that updates gps location every given time

public class mapIt extends MapActivity {

        protected static class DispLocListener implements 
LocationListener {

        @Override
        public void onLocationChanged(Location location) {
        }

        @Override
        public void onProviderDisabled(String provider) {
        }

        @Override
        public void onProviderEnabled(String provider) {
        }

        @Override
        public void onStatusChanged(String provider, int status, 
Bundle extras) {
        }

    }

//need to add an extra field in MapPage.java:
private LocationListener locListenD;

//need to add these 3 lines of code at the end of onCreate method in 
//MapPage.java:
LocationManager lm = (LocationManager)
locListenD = new DispLocListener();
lm.requestLocationUpdates("gps", 6000L, 10.0f, locListenD);

Original issue reported on code.google.com by orkha...@gmail.com on 22 Feb 2010 at 8:37

[GoogleCodeExporter]

CODE REVIEW - Bus Stop Alarm
Date: Feb 22, 2010
File: MapPage.java
Revision: 74
Author: Orkhan Muradov
Reviewer: Derek Cheng

Summary:
This is a first review of MapPage.java. The file contains around 100 lines of 
code as 
of revision 74 and contains code for handling the map page in which the user 
selects 
the destination on a map powered by the Google Maps API. This review will check 
for 
general code formatting, adherence to Java / Android style guidelines and 
general 
object-oriented design principles.

Comments:

CODING STANDARDS

Around lines 44-45, in the declaration of LocationManager lm, no need to 
separate 
into two lines. If the entire statement doesn't fit, put the cast along with 
the 
statement after the equal sign to second line.
onCreate() looks very crowded - place some line breaks between major segments 
of the 
code to separate out the operations performed there. Also, it would help to 
have some 
inline comments in the method to document what each part is doing.
In addEvent(), the variables "Lat" and "Long" should be in longer case. Might 
want to 
change the variable names so it does not conflict with the primitive datatype 
long.    
COMMENTS

Place comments at the very top documenting author, and purpose of the file.
Make sure to document each method using the javadocs style. See Huy's 
BusDbAdapter.java for example.

LOGIC

addEvent() - it might not be a good way to directly "plug in" values into a SQL 
statement like that as it might potentially lead to security problems (SQL 
injections). See next comment.

ERROR HANDLING

Is addEvent guaranteed to be successful? In case of errors, we might need to 
return a 
value indicating failure. Use the insert() or insertOrThrow() method.

CODE DECISIONS

Fields should be made private if possible, so make remember to add in the 
modifiers.

Original comment by chen...@gmail.com on 22 Feb 2010 at 9:22

[GoogleCodeExporter]

In the sentence "In addEvent(), the variables "Lat" and "Long" should be in 
longer
case.", change "longer case" to "lower case".

Original comment by chen...@gmail.com on 22 Feb 2010 at 11:34

[GoogleCodeExporter]

Original comment by chen...@gmail.com on 23 Feb 2010 at 7:27

• Changed state: Fixed

[GoogleCodeExporter]

Original comment by chen...@gmail.com on 23 Feb 2010 at 7:28

• Changed state: Done