1
\$\begingroup\$

I have this short function, it converts a human selected name like "Accelerometer" to "Acc" which a sensor can use:

private final String[] sensorNames = {"Acc", "Gyro", "Magn", "IMU9", "Temp"};

/**
 * Converts human representation of sensor names to movesense names. TODO: better code
 * @param sensorType Human representation of sensor name.
 * @return Short name of the sensor.
 */
private String findShortSensorName(String sensorType) {
    for (int i = 0; i < sensorNames.length; i++) {
        if (sensorType.substring(0,1).equals(sensorNames[i].substring(0,1))) {
            return sensorNames[i];
        }
    }
    return "";
}

Is there a way to make this more efficient or use less code? The return ""; is also bugging me. Thanks for your input!

\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

It's hard to comment really well on this as your problem specification is more than a little vague. Do you really want to do this based only on the first letter - is "AbominableSnowman" to be mapped to "Acc"? Or do you know that the input sensor types are drawn from a specific set of values?

You could make your method (Java doesn't have functions) more succint, for example like this

private String findShortSensorName(String sensorType) {
  for (String sensorName : sensorNames) {
      if (sensorName.startsWith(sensorType.substring(0,1))) {
          return sensorName;
      }
  }
  return "";
}

However, the idiomatic approach for lookup in Java is usually to use a Map, and I'd have thought that if the value isn't found, throwing an Exception may be more appropriate than returning an empty string, though that will depend on your application. As your method doesn't look tied to a particular Java object, it can probably be "static", as well.

My version isn't shorter (it's considerably longer), but it's idiomatic (in my view) and easier to safely extend.

class BadSensorTypeException extends Exception {
// Thrown when a sensor type isn't found in the lookup table

  public BadSensorTypeException(String detailMessage) {
    super(detailMessage);
  }
 }

/**
 * lookup table mapping sensor types to short movesense names
 */
private static final Map<String, String> sensorNames = new HashMap<>();
static {
// static block to initialise the lookup table
  sensorNames.put("Accelerometr", "Acc");
  sensorNames.put("Gyroscope", "Gyro");
  // etc
}

/**
 * Converts human representation of sensor names to movesense names.
 * @param sensorType Human representation of sensor name.
 * @return Short name of the sensor.
 * @throws BadSensorTypeException if the sensor type isn't found
 */
private static String findShortSensorName(String sensorType) throws BadSensorTypeException {
  String shortName = sensorNames.get(sensorType);
  if (shortName != null) {
    return shortName;
  }
  throw new BadSensorTypeException(String.format("%s is not a valid sensor type", sensorType));
}
\$\endgroup\$
1
\$\begingroup\$

If memory is not a constraint here you can create a lookup data structure for example HashMap(key-value data structure) or HashSet.

Once the HashMap/HashSet is populated with the sensor names then after that all your queries will be done in O(1).

considering private final String[] sensorNames = {"Acc", "Gyro", "Magn", "IMU9", "Temp"}; is given or can be read from properties file we can populate a HashMap like below

class Sensor {

    private Map<String, String> map;

    public Sensor() {
        map = new HashMap();
    }

    public void populateSensorNames(String[] sensorNames) {
        for(String str : sensorNames)
            map.put(str.substring(0, 1), str);
    }

    public String findShortSensorName(String sensorType) {
        return map.getOrDefault(sensorType.substring(0, 1), "");
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ I'd be interested to see your suggested solution using a Set. I'm also worried about what happens if the input list of sensorNames is changed to add an entry with the same initial letter as an existing one, which is why my solution does a full string lookup. \$\endgroup\$ Commented Sep 4, 2020 at 7:59
  • \$\begingroup\$ I have used HashMap because I wanted to map start char with the sensor string, with set I can not do that. And I took this approach of checking against the start character because I saw this same comparison in the question, I thought the author wants to reduce the time complexity here. \$\endgroup\$ Commented Sep 4, 2020 at 11:49

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.