user2995349
user2995349

Reputation: 3

Iterating through two arrays, having trouble with indices

I have a question about creating dynamic arrays and iterating through these arrays. I have a method setVoltage that takes as parameters a string and a double value. I needed some way to store these values so I created two arrays. What i need to do is iterate through the String array to see if the string parameter already exists and if it does, to set the corresponding voltage at that index. If it doesn't exist, i need to add the string device to the string array and also add the double voltage to the double array. Can someone check out my code and see what I am missing? I'm running into trouble because I want the array to realize the string is already in the array, or append it to the end of the array but I'm stuck on how to use index variables to achieve this. Thank you!

    public final int sizeArray = 10;
private String[] deviceList = new String[sizeArray]; 
public double[] voltList = new double[sizeArray];

public synchronized void setVoltage(String device, double voltage) {

        int index = 0;
        for(int i = 0; i < deviceList.length; i++ ){
            //if the device name already exists in the device array, overwrite voltage at that index 
            if(this.deviceList[i].equals(device)){
            index = i;
            voltList[index] = voltage;
            }else{
            //set deviceList[i] equal to device, set voltList[i] equal to voltage
            deviceList[i] = device;
            voltList[i] = voltage;
            index++;
            }
        }
}

Upvotes: 0

Views: 65

Answers (4)

Radiodef
Radiodef

Reputation: 37845

This is how I would do it as-is:

public void setOrAppend(String device, double voltage) {

    int index = 0;
    for ( ; index < deviceList.length; i++) {
        if (device.equals(deviceList[index]) || deviceList[index] == null) {
            break;
        }
    }

    if (index == deviceList.length) {
        deviceList = Arrays.copyOf(deviceList, deviceList.length + 1);
        voltList = Arrays.copyOf(voltList, voltList.length + 1);
    }

    deviceList[index] = device;
    voltList[index] = voltage;
}

I would recommend doing the set logic outside the loop. Additionally I would also recommend a Map for this kind of scenario since this kind of logic is simpler. A Map will already do what you are asking automatically (replace if exists otherwise append).

Upvotes: 0

Jason C
Jason C

Reputation: 40336

It sounds like perhaps you want a Map<String,Double> instead. This will let you store voltage values keyed by device name, and you can easily find, insert, and remove devices. See, for example, HashMap:

Map<String,Double> deviceVoltages = new HashMap<String,Double>(); 
deviceVoltages.put("liquifier", 8.4);
deviceVoltages.put("reflarbulator", 900.0);
deviceVoltages.put("liquifier", 13.3); // replaces previous 8.4
System.out.println(deviceVoltages.get("liquifier")); 
deviceVoltages.remove("reflarbulator");

Your example code then reduces to simply:

private Map<String,Double> deviceVoltages = new HashMap<String,Double>(); 

public synchronized void setVoltage(String device, double voltage) {
    deviceVoltages.put(device, voltage);
}

Performance will exceed that of your array, and you will not have a hard-coded limit on number of devices.

See generic Map documentation for links to other types of maps in the JDK.

See also ConcurrentHashMap if a finer synchronization granularity is desired and/or acceptable.

An alternative, if you must use arrays, is to create a Device class that encapsulates all the relevant information about a device and use a single array to simplify your logic, e.g.:

static class Device {
    String name; 
    double voltage;
}

Device[] devices = new Device[10];

You can add new devices by replacing a null element in the array. You can update devices by finding the one with the specified name and changing its voltage field. You can optionally use a List<Device> to avoid hard-coded size limits.

Upvotes: 2

Jean B
Jean B

Reputation: 326

Your code will always overwrite the device if the device name was not found. It would be better if you used a Map but if you are required to use arrays you can try the following

   for(int i = 0; i < deviceList.length; i++ ){
        //if the device name already exists in the device array, overwrite voltage at that index 
        if(this.deviceList[i].equals(device)){
          voltList[i] = voltage;
          break;
        }else if (deviceList[i] == null) {
          //set deviceList[i] equal to device, set voltList[i] equal to voltage
          deviceList[i] = device;
          voltList[i] = voltage;
          break;
        }
    }

You'll have to write some additional code to deal with a full array.

Upvotes: 0

jester
jester

Reputation: 3489

It would be way better to use a Map<String,Double> in this scenario. But if you still want to use two arrays, your current code is not appending to the end of the array. Say if i=0 and deviceList[i] != device , then your code immediately overwrites deviceList[i] with the new value in the else block rather than appending it to the end of the array. To append, you have to move the code in the else part to after the for loop.

Upvotes: 0

Related Questions