Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I am developing a Java program on Windows. I want to access Wi-Fi interface details such as SSID, IP, subnet, enabled or not, connected or not and so on. I am currently trying with using netsh and ipconfig commands.

For example, here is how I get the connected SSID. Is this a good way? Is there any better, easier method? Will this code work with all Windows versions without any problems?

String ssid;
ProcessBuilder builder = new ProcessBuilder(
        "cmd.exe", "/c", "netsh wlan show interfaces");
builder.redirectErrorStream(true);
Process p = builder.start();
BufferedReader r = new BufferedReader(new InputStreamReader(p.getInputStream()));
String line;
while (true) {
    line = r.readLine();
    if (line.contains("SSID")){
        ssid = line.split("\\s+")[3];
        System.out.println(ssid);
        return ssid;
    }
}
share|improve this question

It looks like the solution you posted here was copy-pasted (with some cutting of irrelevant lines) from this SO answer and its original code is quite quick&dirty.

However, here are some observations about it:

  • This will work only on a Windows system.

  • Neither InputStreamReader nor BufferedReader are closed, which is a bad practice.

  • It's not clear how IOException is managed. I suppose that it is rethrown by the wrapping method?

  • while (true) is a bad pattern for loops.

    A better choice would be:

    String line = r.readLine();
    while (line != null) {
      // logic
      line = r.readLine();
    }
    
  • Using line.contains("SSID") seems to be enough to identify the line with the SSID, but it will fail if (accidentally) another line of the process output contains this substring. That should not occur normally, but who ever knows how the parameters are set on the host where it is run? I'd suggest here to split the line in two parts by ':' and check if the trimmed left side equals("SSID").

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.