Re: Reading from hashtables
Some side notes for you that will help code readability (as when you're asking
for help) and your runtime performance and engineering.
christopher_board@yahoo.co.uk wrote:
i [sic] am currently developing a java [sic] application that will read in a CSV
file which includes computer names and mac addresses. I have two
seperate functions within a class. One function is called readFile();
and the other function is called WOL();.
Method names in Java should begin with a lower-case letter and be spelled in
camel case, by well-established convention.
In the readFile() function it reads the CSV file in and then adds the
data into a hashtable. I then want to retrieve a computer name from
the hashtable being ran from the WOL function. However when I output
the hash table to the console it says that the table is null. When I
use the exact same code and place that within the readFile function it
works fine. The code is below:
package remoteshutdown;
import java.awt.Dimension;
import java.awt.Point;
import java.io.*;
import java.net.*;
import java.util.*;
public class WakeOnLan {
String macStr;
Why are your instance variables package-private instead of private?
// String mac;
// String Computer;
Hashtable<String, String> macTable = new Hashtable<String,
String>();
Hashtable isn't your best choice. You don't seem to be making any use of its
synchronization, so HashMap is likely the better choice. Actually, Hashtable
hasn't been the right choice since 1998. You probably should declare the
variable 'macTable' as a Map <String, String> rather than locking it down to
the implementation.
public static final int PORT = 9;
public void readFile() {
try {
PLEASE stop embedding TABs in Usenet posts. It screws up the indentation
something fierce.
// Open the file that is the first
// command line parameter
FileInputStream fstream = new FileInputStream("C:\
\Documents and Settings\\All Users\\Application Data\\Remote Shutdown\
\DHCP Export.csv");
// Get the object of DataInputStream
DataInputStream in = new DataInputStream(fstream);
BufferedReader br = new BufferedReader(new
InputStreamReader(fstream));
DataInputStream is the wrong choice here. Just build your InputStreamReader
run on the FileInputStream. From the Javadocs:
A data input stream lets an application read primitive Java data types
from an underlying input stream in a machine-independent way.
<http://java.sun.com/javase/6/docs/api/java/io/DataInputStream.html>
This is not at all what you are doing. So once again, do not use
DataInputStream to read characters like this.
String strLine;
Once again, avoid embedding implementation ("str") in variable names.
//Read File Line By Line
while ((strLine = br.readLine()) != null) {
Here's an interesting alternative idiom that confines the scope of your 'line'
variable to the loop and no wider, thus making safer code:
for ( String line; (line = br.readLine()) != null; )
System.out.println("strLine is: " + strLine );
If you must use such logging statements without using logging calls, at least
use System.err. Either way, logging statements that you have to remove from
code later are a bad idea.
StringTokenizer st = new StringTokenizer(strLine, ",.");
String Computer = st.nextToken();
if(strLine.contains("."))
{
int firstIndex = strLine.indexOf(".");
int lastIndex = strLine.lastIndexOf(".");
if (firstIndex == lastIndex) {
st.nextToken();
}
else {
st.nextToken();
st.nextToken();
}
}
String mac = st.nextToken();
System.out.println("\t\tComputer : " + Computer+" for
"+mac);
macTable.put((String)Computer, (String)mac);
Why are you casting Strings to Strings in a method that only takes Strings as
srguments?
// Print the content on the console
//Close the input stream
}
System.out.println("CLOSING IN");
in.close();
Interesting choice to close the middle output path, not the lowest or highest.
Note that in this location the close() call is not guaranteed to run. You
risk running around with unclosed streams.
} catch (Exception e) { //Catch exception if any
System.err.println("Error: " + e.toString());
}
}
public void WOL() {
Should be named 'wol'.
String Computer = (String)
mainScreen.lstComputerNames.getSelectedValue();
Others have pointed out that you show no definition for 'mainScreen'.
System.out.println("the mac table has inside: " + macTable);
String wakeComputer = (String)macTable.get("cbo_test2");
Why are you casting the get() result to String? get() already returns a String.
System.out.println("wakeComputer is:" + wakeComputer);
System.out.println("computer variable is: " + Computer);
String ipStr = "255.255.255.255"; //broadcast address
String macStr = "00:30:4F:1C:95:DF"; //CBO_TEST2
try {
byte[] macBytes = getMacBytes(macStr);
byte[] bytes = new byte[6 + 16 * macBytes.length];
for (int i = 0; i < 6; i++) {
bytes[i] = (byte) 0xff;
}
for (int i = 6; i < bytes.length; i += macBytes.length) {
System.arraycopy(macBytes, 0, bytes, i,
macBytes.length);
}
InetAddress address = InetAddress.getByName(ipStr);
DatagramPacket packet = new DatagramPacket(bytes,
bytes.length, address, PORT);
DatagramSocket socket = new DatagramSocket();
socket.send(packet);
socket.close();
System.out.println("Wake-on-LAN packet sent.");
}
catch (Exception e) {
System.out.println("Failed to send Wake-on-LAN packet: +
e");
System.exit(1);
That is too strong a reaction to the failure. In fact, calling System.exit()
from any non-main() is a mistake. What if the invoking code expects to continue?
Do not kill the program from a low-level subroutine.
}
}
private static byte[] getMacBytes(String macStr) throws
IllegalArgumentException {
byte[] bytes = new byte[6];
String[] hex = macStr.split("(\\:|\\-)");
if (hex.length != 6) {
throw new IllegalArgumentException("Invalid MAC
address.");
}
try {
for (int i = 0; i < 6; i++) {
bytes[i] = (byte) Integer.parseInt(hex[i], 16);
}
}
catch (NumberFormatException e) {
throw new IllegalArgumentException("Invalid hex digit in
MAC address.");
}
return bytes;
}
}
You stated that 'readFile()' is called before 'WOL()', but provide no evidence
of that. How are you certain that that order pertains?
Also, since you do nothing in readFile() to notify a caller if it fails, are
you sure that readFile() actually built the Map that you think it did?
Are you sure that wol() is called on the same instance of 'WakeOnLan' on which
'readFile()' was called?
--
Lew