Following function will parse string resp
, which will be like shown in following example. I will get an IP address in the form of a hexadecimal string like "C0A2AA32" and I need to convert that string and save it to a file.
Example of resp
: "This is just a string with ip="2342A022" now I need to parse it"
void WritePublicIPAddressToFile(String resp)
{
//find string in response
String stringToFind = "ip=";
//get index of stringToFind
int indexOfIP = resp.indexOf(stringToFind);
String newString = "";
//find exact ip address which will be after = in stringToFind
for(int i = indexOfIP; i<resp.length(); i++)
{
while(resp.charAt(i++) != '\"');
while(resp.charAt(i) != '\"')
{
newString += resp.charAt(i++);
}
break;
}
int lenNewString = newString.length();
String newIPAddress = "";
//get two bytes from hex string and convert to ipaddress
if(lenNewString == 8) //lenth of ipaddress raw data should be 8
{
long l;
for(int i = 0;i<lenNewString;i+=2)
{
l = Long.parseLong(String.format("%c%c", newString.charAt(i),newString.charAt(i+1)),16);
newIPAddress += String.format("%d.", l);
}
}
StringBuilder sb = new StringBuilder(newIPAddress);
sb.deleteCharAt(sb.length() - 1);//remvoe last '.' in ipaddress
//final ipaddress to save in file
newIPAddress = stringToFind + "\"" + sb.toString() + "\"";
//write IP Address to file
File file = new File("SomeFilePath.txt");
FileWriter fileWriter;
try {
fileWriter = new FileWriter(file);
fileWriter.write(newIPAddress);
fileWriter.flush();
fileWriter.close();
} catch (IOException e2) {
e2.printStackTrace();
}
}
1 Answer 1
For starters you can simplify the first part by utilizing the fact that indexOf can accept a second character indicating start point. This would lead to something like the following:
// Find indices for IP-address
int startIndex = resp.indexOf('\"', resp.indexOf("ip=")) + 1;
int endIndex = resp.indexOf('\"', startIndex);
String ipString = resp.substring(startIndex, endIndex);
Secondly, you can simplify the conversion by utilizing the substring method a little, and improve a little on performance by using StringBuilder
, like in the following:
StringBuilder ipAddress = new StringBuilder();
int lenIpString = ipString.length();
//get two and two bytes from hex string and append to ipaddress
if (lenIpString == 8) {
long ipPart;
for(int i = 0; i < lenIpString; i += 2)
{
ipPart = Long.parseLong(ipString.substring(i, i + 2), 16);
ipAddress.append(String.format("%d.", ipPart));
}
} else {
throw new Exception("IP-string not long enough");
}
Note that this also does something if in the rare case the ipString
is not 8 characters long. This also requires for the method to have an throws Exception
added, and you should probably choose a better exception rather than Exception
.
Finally your write to file can be enhanced to the following:
//write IP Address to file
File file = new File("SomeFilePath.txt");
FileWriter fileWriter;
try {
fileWriter = new FileWriter(file);
fileWriter.write(
String.format("ip=\"%s\"",
ipAddress.substring(0, ipAddress.length() - 1)));
fileWriter.flush();
fileWriter.close();
} catch (IOException exception) {
exception.printStackTrace();
}
Finally some general notes on coding style:
Consider splitting into three functions – These three code blocks could be extracted into (two or three) separate functions, where one serves as the output to the next. This would follow the single-responsibility principle of coding. One variant could change this function into:
void WritePublicIPAddressToFile(String resp, String filename) { String ipAddress = findAndConvertIPAddress(resp); if (!ipAddress.isEmpty()) { writeIPAddressToFile(filename, ipAddress); } }
Here I assume that the
findAndConvertIPAddress()
would return the empty string if the IP-address is not found as expected, and use that to bypass the file write if necessary. Now this function has a much clearer responsibility of finding and writing that IP-address to the file. No need to read lots of code to understand whats happening.Use
StringBuilder
instead ofString
concatenation – In general try to avoid building new strings to a minimum, and useStringBuilder
when building new strings, or a variation overString.format()
.- Consider having the filename as a parameter – Your code might be constructed specific for Code Review, but with a name like
WritePublicIPAddressToFile
I would expect thatfilename
was one of the parameters. - Avoid using
l
as variable name – Especially in combination withi
or1
, it can be really confusing to use lower-caseL
as a variable name. Use more descriptive names instead.