Re: Reading from hashtables
On 4 Feb, 14:43, Lew <l...@lewscanon.com> wrote:
Some side notes for you that will help code readability (as when you're as=
king
for help) and your runtime performance and engineering.
christopher_bo...@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 o=
f its
synchronization, so HashMap is likely the better choice. Actually, Hash=
table
hasn't been the right choice since 1998. You probably should declare th=
e
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 indentatio=
n
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(fstre=
am);
BufferedReader br = new BufferedReader(new
InputStreamReader(fstream));
DataInputStream is the wrong choice here. Just build your InputStreamRe=
ader
run on the FileInputStream. From the Javadocs:> A data input stream let=
s 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 'l=
ine'
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 le=
ast
use System.err. Either way, logging statements that you have to remove =
from
code later are a bad idea.
StringTokenizer st = new Str=
ingTokenizer(strLine, ",.");
String Computer = st.nextToken(=
);
if(strLine.contains("."))
{
int firstIndex =
= strLine.indexOf(".");
strLine.lastIndexOf(".");
== lastIndex) {
t.nextToken();
}
else {
st.nextToken();
st.nextToken();
}
}
String mac = st.nextToken();
System.out.println("\t\tComputer =
: " + Computer+" for
"+mac);
macTable.put((String)Computer, (S=
tring)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 high=
est.
Note that in this location the close() call is not guaranteed to run. Y=
ou
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: " + macTab=
le);
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:" + wakeCompu=
ter);
System.out.println("computer variable is: " + Com=
puter);
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.leng=
th];
for (int i = 0; i < 6; i++) {
bytes[i] = (byte) 0xff;
}
for (int i = 6; i < bytes.length; i += macBy=
tes.length) {
System.arraycopy(macBytes, 0, bytes, i,
macBytes.length);
}
InetAddress address = InetAddress.getByName(ip=
Str);
DatagramPacket packet = new DatagramPacket(byt=
es,
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 p=
acket: +
e");
System.exit(1);
That is too strong a reaction to the failure. In fact, calling System.e=
xit()
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 evid=
ence
of that. How are you certain that that order pertains?
Also, since you do nothing in readFile() to notify a caller if it fails, a=
re
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 w=
hich
'readFile()' was called?
--
Lew- Hide quoted text -
- Show quoted text -- Hide quoted text -
- Show quoted text -- Hide quoted text -
- Show quoted text -- Hide quoted text -
- Show quoted text -
Thank you for your help. I have now resolved this issue