Skip to content

Minor fixes

  • General:

    • "cleanObject" methods: are they used anywhere? I'm not sure they're really useful.
    • Function names: normally, there shouldn't be a space between the function name and the opening parenthesis of the input list.
    • Variable names: in my opinion, we should remove the "all..." and add an "s" at the end of variable names to indicate that there are multiple.

  • CzmlFile:

    • Why use abstractPrimaryObjects and not the interface directly? If it's for the "getId", maybe we should add getId() to the interface.
    • Path management: we can discuss how to improve this, maybe by using the File class directly (particularly to retrieve the parent, etc.).
    • Builder:
      • Why do some objects have "Display" in class names and others don't? This is more of a general remark.

  • CzmlSecondaryObject:

    • Date conversion: duplicates.

  • Header:

    • Why are MASTER_CLOCK and TIME_SCALE static?
    • Management of DEFAULT_ROOT and DEFAULT_RESOURCES: We should see if there's a cleaner way to handle this.
    • What is the pathToExternalResourceFolder used for?

  • Satellite:

    • Extends AbstractPrimaryObject implements CzmlPrimaryObject → This applies throughout the code.
    • Constructor:
      • Okay, I understand why the header variables are static; in my opinion, they should be passed as arguments.
      • I also think it's not a good practice to propagate during the construction of the Satellite object.
      • At the very least, encapsulate it in a function.
      • The try/catch can be avoided. o "getOrbits()": I’m not sure how often it's used, but it's better to use S/C states to get the dates, PVs, etc. An S/C state doesn't always contain an Orbit; it may contain an AbsolutePV instead.
    • Clean up unused getters and setters. o Period management: As mentioned earlier, some S/C states don’t have an orbit. In such cases, we should return an exception if the user tries to display a period.
    • We need to figure out something for all these lists!
      • Example: the groundtrack constructor  Conversion S/C states → Cartesians.  Creation of the date list.  Loop over Cartesians.
      • Convert Cartesian → Vector3D.
      • Project using the date list.
      • Convert Vector3D → Cartesian. → You could do all of this in one line by looping directly over the S/C states.  Perhaps storing the julianDates would be useful so that you don't have to recreate them every time.

  • Orientation:

    • Constructor:
      • There's something we haven't discussed yet that we'll need to address—the concept of DefaultDataContext or not.
      • Here we’re forcing ITRF, but if, for example, I want to display an orbit around Mars, can't I use a dedicated bodyFrame? o Are julianDates really useful to pass around? Or do we only need them when writing into the Czml?
    • Distinction between singleAttitude and multipleAttitudes: can't we handle all of this with lists of size 1 for the single case?

  • AttitudePointing:

    • Constructors:
      • Comments?
      • Direction: lineOfSight? Well specified that it's in the S/C state frame.
      • Create a function for building the "projectedAttitudes"?
      • You loop over an attitude list, but then use state.getAttitude()
      • If it's the same, maybe the attitude list is unnecessary.
      • What’s the difference between currentCartesian and currentSatellitePosition?
    • projectedAttitudes: • The name is misleading—it sounds like attitudes, but they're geodetic points.
      • If they're only used to create pointOnBody, we probably don't need to store them, right?

  • AbstractPointOnBody:

    • The class isn't abstract…
    • It's singular even though we may have multiple points: "pathOnBody" or "pointsOnBody"?
    • You don't need to go through the topocentric frame: bodyShape.transform(GeodeticPoint) is enough.
    • Couldn't we actually store a TimePosition?

  • CzmlModel:

    • The class isn't abstract, but it's in CzmlAbstractObjects.

  • Cylinder:

    • Constructor: "this.topRadius = length * FastMath.tan(angleOfAperture);" Careful, I think you're taking the tangent of an angle in degrees.

  • Position:

    • Default value of a double = 0 / you can remove default values if you want.

  • CovarianceDisplay:

    • Constructor: simplify the second one.

  • FieldOfObservation:

    • Constructor: simplify by looping over the S/C states + be careful, "currentFovBody" is wrongly named.

• TimePosition: o Here we use a date delta to create JulianDate, even though we already have the list of Julian dates somewhere.