Nsryjdtyk

I have 2 Classes, the first one is only used as Container,
and the second Class Splits a GpsString, Converts it to suitable data types and returns the Container Class.

The String to analyze looks about like this:

("$GPGGA,120003.533,7926.184,N,02222.419,W,1,12,1.0,88.5,M,0.0,M,,*71")

Now i want to know if this is good practice in ObjectOriented Programming,
or if there is a better way for this Problem.

final class GpsData {

    int Hours;

    int Minutes;

    double Seconds;

    double Lattitude;

    char DirectionLattitude; // North or South

    double Longitude;

    char DirectionLongitude; // North or South

    int Quality;

    int Satelites;

}





class ProtocolAnalyzer {

    private GpsData AnalyzedData = new GpsData();

    boolean AnalyzeData(String GpsData){

        GpsData Gps = new GpsData();

        int Attribute = 0;

        StringBuilder WorkString = new StringBuilder();

        int beginIndex = 0;

        int length = GpsData.length();

        for (int i = 0;i<=length;i++){

            if(i == GpsData.length() ||

            GpsData.charAt(i) == ','){

            String Data = GpsData.substring(beginIndex,i);

            boolean bool = ConvertData(Data,Attribute,Gps);

            if(!bool) return false;

            beginIndex = i + 1;

            Attribute ++;

        }

    }

    setAnalyzedData(Gps);

    return true;

}

private boolean ConvertData(String Data,int Attribute,GpsData Gps){

boolean b = false;

switch (Attribute) {

    case 0: // §GPGGA

        if(Data.equals("$GPGGA")) {

            b = true;

        }

        break;

    case 1: //HHMMSS.sss

        if(Data.length() == 10) {

            b = true;

            String split = Data.substring(0,2);

            Gps.Hours = Integer.parseInt(split);

            split = Data.substring(2,4);

            Gps.Minutes = Integer.parseInt(split);

            split = Data.substring(4,10);

            Gps.Seconds = Double.parseDouble(split);

        }

        break;

    case 2: //BBBB.BBBB

        if (Data.length() != 0) {

            b = true;

            Gps.Lattitude = Double.parseDouble(Data);

        }

        break;

    case 3: //b

        if (Data.length() != 0) {

            char DirLat = Data.charAt(0);

            if (DirLat == 'N' ||

                    DirLat == 'S') {

                b = true;

                Gps.DirectionLattitude = DirLat;

            }

        }

        break;

    case 4: //LLLLL.LLLL

        if (Data.length() != 0) {

            b = true;

            Gps.Longitude = Double.parseDouble(Data);

        }

        break;

    case 5: //l

        if (Data.length() != 0) {

            char DirLong = Data.charAt(0);

            if (DirLong == 'E' ||

                    DirLong == 'W') {

                b = true;

                Gps.DirectionLongitude = DirLong;

            }

        }

        break;

    case 6: //Q

        if (Data.length() != 0) {

            int Quality = Integer.parseInt(Data);

            Gps.Quality = Quality;

            if(Quality != 0) {

                b = true;

            }

        }

        break;

    case 7: //NN

        if (Data.length() != 0) {

            int Satellites = Integer.parseInt(Data);

            Gps.Satelites = Satellites;

            if(Satellites > 3) {

                b = true;

            }

        }

        break;

}

return b;

}



private void setAnalyzedData(GpsData LogData) {

    this.AnalyzedData = (LogData);

}



GpsData getAnalyzedData() {

    return AnalyzedData;

}

}

First, please please please use Java naming conventions to aid readability. Methods and local variables like int Hours should be in camelCase and not CapitalCase which is for classes. Statements like this:

boolean AnalyzeData(String GpsData){

are especially confusing. Is GpsData referring to the local variable, or the class? That can get very nasty very quickly. Please fix this before you do anything else.

final class GpsData {

This does not do what I suspect you think it does. A final declaration for a class just means it can't be further extended. Its data can still be modified. To properly encapsulate the properties of GpsData you should declare each of those properties as private and provide public constructors, setters, and getters as appropriate.