Skip to main content
GameDev.net gamedev.net
🔒 Locked

Iterating Through File Throws "NULL POINTER" Error

Started by RLS0812 Dec 11, 2012 at 6:05 AM 10 replies 3.7k views
Original Post
RLS0812
RLS0812
I am getting very frustrated with this. This is literally "text book" on how I am to iterate threw a text file - YET it throws a NULL POINTER error when it reaches the end of the file - which it shouldn't do.

Some_File.file

STONE|99
GRASS|2
SAND|67

Code

try{
FileInputStream fstream = new FileInputStream("Some_File.file");
DataInputStream is = new DataInputStream(fstream);
BufferedReader br = new BufferedReader(new InputStreamReader(is));
String Line;
while ( ( Line = br.readLine() ) != null ) { // This throws NULL POINTER after done with file
String[] Temp = Line.split("\\|");
System.out.println(Temp[0].trim() + " " + Temp[1].trim() + " " + Line);
}
}
I cannot remember the books I've read any more than the meals I have eaten; even so, they have made me. ~ Ralph Waldo Emerson
SpringDev
SpringDev
Are you sure it throws a null pointer exception on that part of the code? It looks okay and it actually works as well after trying to compile it. I think we indeed need a stack trace to track the problem.
rip-off
rip-off
Can you post a complete, minimal example?

The following does not reproduce the error for me:

import java.io.*;


public class Test {
public static void main(String [] args) throws Exception {
FileInputStream fstream = new FileInputStream("Some_File.file");
try {
DataInputStream is = new DataInputStream(fstream);
BufferedReader br = new BufferedReader(new InputStreamReader(is));
String Line;
while ( ( Line = br.readLine() ) != null ) {
String[] Temp = Line.split("\\|");
System.out.println(Temp[0].trim() + " " + Temp[1].trim() + " " + Line);
}
} finally {
fstream.close();
}
}
}
RLS0812
RLS0812
Stack Trace: ( Points To Line 273 )

13:30:29 [INFO] RLS0812 issued server command: /mc_web item withdraw
13:30:29 [INFO] STONE 99 STONE|99
13:30:29 [INFO] GRASS 2 GRASS|2
13:30:29 [INFO] SAND 67 SAND|67
13:30:29 [SEVERE] null
org.bukkit.command.CommandException: Unhandled exception executing command 'mc_w
eb' in plugin MC_Web v1.0E-8
at org.bukkit.command.PluginCommand.execute(PluginCommand.java:46)
at org.bukkit.command.SimpleCommandMap.dispatch(SimpleCommandMap.java:18
6)
at org.bukkit.craftbukkit.CraftServer.dispatchCommand(CraftServer.java:5
02)
at net.minecraft.server.NetServerHandler.handleCommand(NetServerHandler.
java:985)
at net.minecraft.server.NetServerHandler.chat(NetServerHandler.java:903)
at net.minecraft.server.NetServerHandler.a(NetServerHandler.java:858)
at net.minecraft.server.Packet3Chat.handle(Packet3Chat.java:44)
at net.minecraft.server.NetworkManager.b(NetworkManager.java:290)
at net.minecraft.server.NetServerHandler.d(NetServerHandler.java:113)
at net.minecraft.server.ServerConnection.b(SourceFile:39)
at net.minecraft.server.DedicatedServerConnection.b(SourceFile:30)
at net.minecraft.server.MinecraftServer.r(MinecraftServer.java:595)
at net.minecraft.server.DedicatedServer.r(DedicatedServer.java:222)
at net.minecraft.server.MinecraftServer.q(MinecraftServer.java:493)
at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:426)
at net.minecraft.server.ThreadServerApplication.run(SourceFile:856)
Caused by: java.lang.NullPointerException
at Me.RLS0812.MC_Web.Main.onCommand(Main.java:273)
at org.bukkit.command.PluginCommand.execute(PluginCommand.java:44)
... 15 more
13:30:32 [INFO] RLS0812 lost connection: disconnect.quitting
13:30:32 [INFO] Connection reset



Source Code:


package Me.RLS0812.MC_Web;


import java.io.BufferedReader
import java.io.BufferedWriter;
import java.io.DataInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileWriter;
import java.io.IOException;
import java.io.InputStreamReader;
import java.text.DecimalFormat;
import java.util.Random;
import com.iCo6.system.Account;
import com.iCo6.system.Accounts;
import org.bukkit.ChatColor;
import org.bukkit.Material;
import org.bukkit.command.Command;
import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player;
import org.bukkit.inventory.ItemStack;
import org.bukkit.plugin.java.JavaPlugin;
public class Main extends JavaPlugin{
// Import Money
boolean Imoney;
// Export Money
boolean Emoney;
// Maximum Money Per Day
int Mmoney; // Not Used In Plugin - Server Side Use Only
// Money Name
String Money_Name;
// Import Items
boolean Iitems;
// Export Items
boolean Eitems;
// Maximum Items Per Day
int Mitems; // Not Used In Plugin - Server Side Use Only
// Banned Player Deletion
boolean Bplayer;
public void onEnable(){
File file = new File("MC_Web");
if (!file.exists()){
System.out.println("\n** MC_Web First Time Set Up **");
file.mkdir();
File file1 = new File ("MC_Web/Users");
file1.mkdir();
File cfg = new File ("MC_Web/MC_Web.config");
try {
cfg.createNewFile();
FileWriter fileWritter = new FileWriter("MC_Web/MC_Web.config",true);
BufferedWriter bufferWritter = new BufferedWriter(fileWritter);
bufferWritter.write("Allow_Money_Export=true\n");
bufferWritter.write("Allow_Money_Import=true\n");
bufferWritter.write("Max_Money_Per_Day=-1\n");
bufferWritter.write("Money_Name=Gold\n");
bufferWritter.write("Allow_Material_Export=true\n");
bufferWritter.write("Allow_Material_Import=true\n");
bufferWritter.write("Max_Material_Per_Day=-1\n");
bufferWritter.write("Auto_Delete_Banned_Player_Account=false\n");
bufferWritter.close();
}
catch (IOException e) {
System.out.println("\n** MC_Web Error Creating Config File **");
}}
System.out.println("\n** MC_Web Loading Config **");
File cfg = new File ("MC_Web/MC_Web.config");
if (!cfg.exists()){
System.out.println("\n** MC_Web Error Finding Config File - Creatinng New Config File **");
try {
cfg.createNewFile();
FileWriter fileWritter = new FileWriter("MC_Web/MC_Web.config",true);
BufferedWriter bufferWritter = new BufferedWriter(fileWritter);
bufferWritter.write("Allow_Money_Export=true\n");
bufferWritter.write("Allow_Money_Import=true\n");
bufferWritter.write("Max_Money_Per_Day=-1\n");
bufferWritter.write("Money_Name=Gold\n");
bufferWritter.write("Allow_Material_Export=true\n");
bufferWritter.write("Allow_Material_Import=true\n");
bufferWritter.write("Max_Material_Per_Day=-1\n");
bufferWritter.write("Auto_Delete_Banned_Player_Account=false\n");
bufferWritter.close();
}
catch (IOException e) {
System.out.println("\n** MC_Web Error Creating New Config File **");
}}

try{
FileInputStream fstream = new FileInputStream("MC_Web/MC_Web.config");
DataInputStream is = new DataInputStream(fstream);
BufferedReader br = new BufferedReader(new InputStreamReader(is));
String Line;
int x = 0;
// If you add config lines, change X and case
while (x != 8) {
Line = br.readLine().trim();
String[] Temp = Line.split("=");
switch(x){
case 0:Imoney = Boolean.parseBoolean(Temp[1]);break;
case 1:Emoney = Boolean.parseBoolean(Temp[1]);break;
case 2:Mmoney = Integer.parseInt(Temp[1]);break;
case 3:Money_Name = Temp[1];break;
case 4:Iitems = Boolean.parseBoolean(Temp[1]);break;
case 5:Eitems = Boolean.parseBoolean(Temp[1]);break;
case 6:Mitems = Integer.parseInt(Temp[1]);break;
case 7:Bplayer = Boolean.parseBoolean(Temp[1]);break;
default:System.out.println("\n** MC_Web Error - Extra Config Lines **\n ** Line " + (x+1) + ": " + Line + " **");
}
x += 1;
}
is.close();
br.close();
}
catch (Exception e){
System.out.println(e + "\n");
System.out.println("\n** MC_Web Error Setting Configurations From File **");
}}
public void onDisable(){}
// #### Book Mark 1 ####
// TODO Deposit $ - Withdraw $
// TODO Banned Player Deletion
public void error(Player x){
x.sendMessage(ChatColor.GOLD + "** MC_Web Help **");
x.sendMessage(ChatColor.GOLD + "@ Account Create <Password_Here> <NickName_Here>");
x.sendMessage(ChatColor.GOLD + "This will create an account for you");
x.sendMessage(ChatColor.GOLD + "@ Account Reset <Password_Here>");
x.sendMessage(ChatColor.GOLD + "This will reset your password");
x.sendMessage(ChatColor.GOLD + "@ Item Deposit");
x.sendMessage(ChatColor.GOLD + "This will deposit all the same type of items as in your hand,\n from your inventory to your 'in box' ");
x.sendMessage(ChatColor.GOLD + "@ Item Withdraw");
x.sendMessage(ChatColor.GOLD + "This will deposit all items from your 'out box', to your inventory");
x.sendMessage(ChatColor.GOLD + "@ Money Deposit <Ammount_Here>");
x.sendMessage(ChatColor.GOLD + "This will deposit money into your account ");
x.sendMessage(ChatColor.GOLD + "@ Money Withdraw <Ammount_Here>");
x.sendMessage(ChatColor.GOLD + "This will withdraw money from your account");
}
public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) {

if (sender instanceof Player) {
if (cmd.getName().equalsIgnoreCase("mc_web")){
Player pl = (Player) sender;
String Name = pl.getName();
Account account = new Accounts().get(Name);
Double Money = account.getHoldings().getBalance();
DecimalFormat Trun = new DecimalFormat("0.00");
if (args.length < 2 ){
error(pl);
}
// Account Creation
else if ( args[0].equalsIgnoreCase("account") && args[1].equalsIgnoreCase("create") ){
if ( args.length < 3 || args.length < 4){
error(pl);
}
else{
File file = new File("MC_Web/Users/" + Name);
if (file.exists() ){
pl.sendMessage(ChatColor.AQUA + "Error - Account '" + Name + "' allready exists");
}
else{
file.mkdir();
String Temp = "MC_Web/Users/" + Name;
File file1 = new File(Temp + "/Blarg.acc");
File file2 = new File(Temp + "/Account.acc");
File file3 = new File(Temp + "/In.box");
File file4 = new File(Temp + "/Out.box");
try{
file1.createNewFile();
file2.createNewFile();
FileWriter fileWritter1 = new FileWriter("MC_Web/Users/" + Name +"/Account.acc",false);
BufferedWriter bufferWritter1 = new BufferedWriter(fileWritter1);
bufferWritter1.write("0.0");
bufferWritter1.close();
file3.createNewFile();
file4.createNewFile();
FileWriter fileWritter = new FileWriter(Temp + "/Blarg.acc",true);
BufferedWriter bufferWritter = new BufferedWriter(fileWritter);
bufferWritter.write(args[3] + "\n");
bufferWritter.write(args[2] + "\n");
Random rand = new Random();
bufferWritter.write("" + rand.nextInt(999999999) + "\n"); // User Number
bufferWritter.close();
pl.sendMessage(ChatColor.GOLD + "Account Created");
pl.sendMessage(ChatColor.GOLD + "Username: " + Name);
pl.sendMessage(ChatColor.GOLD + "Password: " + args[2]);
pl.sendMessage(ChatColor.GOLD + "Nickname: " + args[3]);
System.out.println("\n ** MC_Web User Account Created '" + Name + " '");
}
catch (IOException e) {
pl.sendMessage(ChatColor.AQUA + "Error - Account Failed To Create");
System.out.println("\n ** MC_Web Error - Account '" + Name + "' Failed To Create");
}}}}
// Password Reset
else if(args[0].equalsIgnoreCase("account") && args[1].equalsIgnoreCase("reset")){
if ( args.length < 3 ){
error(pl);
}
else{
boolean pf = true;
String pnn = "not_me";
String pwr = args[2];
Random rand = new Random();
String pwn = "" + rand.nextInt(999999999);
try{
FileInputStream fstream = new FileInputStream("MC_Web/Users/" + Name +"/Blarg.acc");
DataInputStream is = new DataInputStream(fstream);
BufferedReader br = new BufferedReader(new InputStreamReader(is));
pnn = br.readLine();
is.close();
br.close();
}
catch (IOException e){
pl.sendMessage(ChatColor.AQUA + "Error - Account Failed To Reset");
System.out.println(e);
System.out.println("\n ** MC_Web Error - Account '" + Name + "' Failed To Reset - Failure Type 1");
pf = false;
}
try{
if (pf){
FileWriter fileWritter = new FileWriter("MC_Web/Users/" + Name +"/Blarg.acc",false);
BufferedWriter bufferWritter = new BufferedWriter(fileWritter);
bufferWritter.write(pnn +"\n");
bufferWritter.write(pwr +"\n");
bufferWritter.write(pwn +"\n"); // User Number
bufferWritter.close();
pl.sendMessage(ChatColor.GOLD + "Account Updated");
System.out.println("\n ** MC_Web User Account Updated '" + Name + " '");
}
}
catch (IOException e){
pl.sendMessage(ChatColor.AQUA + "Error - Account Failed To Reset");
System.out.println(e);
System.out.println("\n ** MC_Web Error - Account '" + Name + "' Failed To Reset - Failure Type 2");
}}}
// Item Deposit
else if(args[0].equalsIgnoreCase("item") && args[1].equalsIgnoreCase("deposit") ){
if (Eitems == true){
ItemStack It[] = pl.getInventory().getContents();
ItemStack Ih = pl.getItemInHand();
String Mih = "" + Ih.getType(); // Deposit Type
int Amm = 0; // Deposit Amount
if (Ih.getType() != Material.AIR){
for (int i = 0; i < It.length; i++){
if ( It != null ){
if ( Ih.getType() == It.getType()){
Amm += It.getAmount();
pl.getInventory().setItem( i , new ItemStack(Material.AIR, 0) );
}}}
try{
FileWriter fileWritter = new FileWriter("MC_Web/Users/" + Name +"/In.box",true);
BufferedWriter bufferWritter = new BufferedWriter(fileWritter);
bufferWritter.write(Mih + "|" + Amm + "\n");
bufferWritter.close();
pl.sendMessage(ChatColor.GOLD + "Items Deposited");
}
catch (IOException e){
pl.sendMessage(ChatColor.AQUA + "Error - Deposit Failed");
System.out.println(e);
System.out.println("\n ** MC_Web Error - Account '" + Name + "' Failed To Deposit Items");
}}
else{
pl.sendMessage(ChatColor.AQUA + "Error - You must be hoding something.");
}}
else{
pl.sendMessage(ChatColor.AQUA + "Error - You are not allowed to deposit items");
}}
// Item Withdraw
else if (args[0].equalsIgnoreCase("item") && args[1].equalsIgnoreCase("withdraw") ){
if (Iitems == true){
try{
FileInputStream fstream = new FileInputStream("MC_Web/Users/" + Name + "/Out.box");
DataInputStream is = new DataInputStream(fstream);
BufferedReader br = new BufferedReader(new InputStreamReader(is));
String Line;
while ( ( Line = br.readLine().trim() ) != null ) { // This throws NULL POINTER after done with file
String[] Temp = Line.split("\\|");
System.out.println(Temp[0] + " " + Temp[1] + " " + Line);
ItemStack item = new ItemStack(Material.getMaterial(Temp[0]),Integer.parseInt(Temp[1]));
pl.getLocation().getWorld().dropItemNaturally(pl.getLocation(), item);

}
is.close();
br.close();
FileWriter fileWritter = new FileWriter("MC_Web/Users/" + Name +"/Out.box",false);
BufferedWriter bufferWritter = new BufferedWriter(fileWritter);
bufferWritter.write("");
bufferWritter.close();
pl.sendMessage(ChatColor.GOLD + "Items Withdrawn");
}
catch (IOException e){
pl.sendMessage(ChatColor.AQUA + "Error - Item Withdraw");
System.out.println(e);
System.out.println("\n ** MC_Web Error - Account '" + Name + "' Failed To Withdraw Items");
}}
else{
pl.sendMessage(ChatColor.AQUA + "Error - You are not allowed to withdraw items");
}}
// Money Deposit
else if ( args[0].equalsIgnoreCase("money") && args[1].equalsIgnoreCase("deposit") ){
if (args.length < 3 ){
error(pl);
}
else{
if(Emoney == true){
double Mon = 0.;
double Bal = 0.;
boolean pf = true;
try{
Mon = Double.valueOf(args[2]).doubleValue();
FileInputStream fstream = new FileInputStream("MC_Web/Users/" + Name +"/Account.acc");
DataInputStream is = new DataInputStream(fstream);
BufferedReader br = new BufferedReader(new InputStreamReader(is));
Bal = Double.valueOf(br.readLine()).doubleValue();
is.close();
br.close();
}
catch (Exception e){
pf = false;
pl.sendMessage(ChatColor.AQUA + "Error - Invalad Number Conversion");
}
if ( pf ){
int Enough_Money = Double.compare(Mon,Money);
if (Enough_Money < 0 ) {
try{
Bal += Mon;
FileWriter fileWritter = new FileWriter("MC_Web/Users/" + Name +"/Account.acc",false);
BufferedWriter bufferWritter = new BufferedWriter(fileWritter);
bufferWritter.write(Bal +"\n");
bufferWritter.close();
account.getHoldings().subtract(Mon);
pl.sendMessage(ChatColor.GOLD + ""+ Mon + " " + Money_Name + " has been deposited");
pl.sendMessage(ChatColor.GOLD + "Account balance is now " + Trun.format(Bal) + " " + Money_Name);
}
catch (IOException e){
pl.sendMessage(ChatColor.AQUA + "Error - Making Deposit");
System.out.println("\n ** MC_Web Error - '" + Name + "' deposit failed");
}}
else{
pl.sendMessage(ChatColor.AQUA + "Too Much To Deposit: " + Trun.format(Mon) + " " + Money_Name);
}}
}
else{
pl.sendMessage(ChatColor.AQUA + "Error - You are not allowed to deposit money");
}}}
// Money Withdraw
else if ( args[0].equalsIgnoreCase("money") && args[1].equalsIgnoreCase("withdraw") ){
if (args.length < 3 ){
error(pl);
}
else{
if(Imoney == true){
double Mon = 0.;
double Bal = 0.;
boolean pf = true;
try{
Mon = Double.valueOf(args[2]).doubleValue();
FileInputStream fstream = new FileInputStream("MC_Web/Users/" + Name +"/Account.acc");
DataInputStream is = new DataInputStream(fstream);
BufferedReader br = new BufferedReader(new InputStreamReader(is));
Bal = Double.valueOf(br.readLine().trim() ).doubleValue();
is.close();
br.close();

}
catch (Exception e){
pf = false;
pl.sendMessage(ChatColor.AQUA + "Error - Invalad Number Conversion");
}
if ( pf ){
int Enough_Money = Double.compare(Bal,Mon);
if (Enough_Money > 0 ){
try{
Bal -= Mon;
FileWriter fileWritter = new FileWriter("MC_Web/Users/" + Name +"/Account.acc",false);
BufferedWriter bufferWritter = new BufferedWriter(fileWritter);
bufferWritter.write(Bal +"\n");
bufferWritter.close();
account.getHoldings().add(Mon);
pl.sendMessage(ChatColor.GOLD + ""+ Mon + " " + Money_Name + " has been withdrawn");
pl.sendMessage(ChatColor.GOLD + "Account balance is now " + Trun.format(Bal) + " " + Money_Name);
}
catch (IOException e){
pl.sendMessage(ChatColor.AQUA + "Error - Making Withdraw");
System.out.println("\n ** MC_Web Error - '" + Name + "' withdraw failed");
}}
else{
pl.sendMessage(ChatColor.AQUA + "Too Much To Withdraw: " + Trun.format(Mon) + " " + Money_Name);
}
}}
else{
pl.sendMessage(ChatColor.AQUA + "Error - You are not allowed to withdraw money");
}}}
// Catch
else{
error(pl);
}}}
return true;
}}
I cannot remember the books I've read any more than the meals I have eaten; even so, they have made me. ~ Ralph Waldo Emerson
rip-off
rip-off
I repeat, can you post a complete, minimal example?

Or is that really as minimal as you can get it without removing the behaviour?
RLS0812
RLS0812
Line 273 - The issue is in the middle of a nested nest, if I try to extract it ( and still have it functional ) I'll break it wacko.png
I cannot remember the books I've read any more than the meals I have eaten; even so, they have made me. ~ Ralph Waldo Emerson
fae
fae
You have "trim" in your while loop which wasn't there in the original post:


while ( ( Line = br.readLine().trim() ) != null ) {
...
}


On the last iteration the trim is called on a null String, which causes the NullPointerException. Move the trim inside the loop as you had in your original example.
RLS0812
RLS0812

You have "trim" in your while loop which wasn't there in the original post:


while ( ( Line = br.readLine().trim() ) != null ) {
...
}


On the last iteration the trim is called on a null String, which causes the NullPointerException. Move the trim inside the loop as you had in your original example.

Why didn't I think of that - thank you very much, this fixed the issue smile.png
I cannot remember the books I've read any more than the meals I have eaten; even so, they have made me. ~ Ralph Waldo Emerson
rip-off
rip-off

The issue is in the middle of a nested nest, if I try to extract it ( and still have it functional ) I'll break it
[/quote]
Sure you can. First off, I meant create a minimal example by copying the code elsewhere, not by modifying it in-place (though you're using version control, right?). Once you've it copied, it should be easy to extract a small program demonstrating the problem - see my code above.

When posting on a public forum, it is easiest if one can simply run the code and debug the code, not just inspect it. Obviously, every dependency is more work for those who wish to run your code. An example like my earlier one can be run by pretty much anyone with a minimal JDK install.

Also, the very act of making this minimal example focuses your mind on what the code is actually doing - it removes all the confusing irrelevant code that is not causing the issue.

Thirdly, it will be much easier to debug the code in isolation rather than mixed in with a bunch of other stuff. Had you used a debugger this source of this error would have been quickly found.

And a final note, make sure the minimal example actually does reproduce the error. The code in your original post did not, once surrounded by sufficient scaffolding to actually run.
rip-off
rip-off
Given that you appear to be loading this plugin into a server, you should carefully consider the security of the host when handling any user-submitted data. For example, what if a malicious user creates a user name containing directory climbing references, such as "../anotheruser". Can the user submit newline delimited text to the server? If so, the user might be able to subvert your file format by inserting newlines in unexpected locations.

Do not rely on Minecraft to validate anything for you - a malicious user might have a modified client or insert a proxy between the client and server which might allow them to break lots of rules you might expect to be enforced.

Failure to correctly handle security can not only cause the loss of the plugin data, but in extreme cases could allow other data on the host to be compromised or deleted, worst case scenario the host might be remotely compromised.

Another issue is data integrity, if this is a serious project you should strongly consider using an existing robust database system rather than manually implementing an ad-hoc database on top of flat files.

Finally, your code is extremely confusing because you've packed all the logic in one enormous method. Consider introducing private helper methods. The dispatch method could look like this:

public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) {
if (!(sender instanceof Player)) {
logError("Invalid sender: " + sender);
return whatever;
}

Player player = (Player) sender;
String commandName = cmd.getName();
if (!commandName.equalsIgnoreCase("mc_web")) {
printUsage(player, "Unrecognised command: " + commandName);
return whatever;
}


if (args.length < 2) {
printUsage(player, "Not enough arguments: " + args.length);
return whatever;
}

String target = args[0];
String verb = args[1];

if (target.equalsIgnoreCase("account")) {
if (verb.equalsIgnoreCase("create") ) {
handleCreateAccount(player, /* ... */);
return whatever;
} else if (verb.equalsIgnoreCase("reset")) {
handleResetAccount(player, /* ... */);
return whatever;
}
} else if (target.equalsIgnoreCase("item")) {
if (verb.equalsIgnoreCase("deposit")) {
handleDepositItem(player, /* ... */);
return whatever;
} else if (verb.equalsIgnoreCase("withdraw") ){
handleWithdrawItem(player, /* ... */);
return whatever;
}
} else if (target.equalsIgnoreCase("money")) {
if(verb.equalsIgnoreCase("deposit")) {
handleDepositMoney(player, /* ... */);
return whatever;
} else if(verb.equalsIgnoreCase("withdraw")) {
handleWithdrawMoney(player, /* ... */);
return whatever;
}
}

printUsage(player, "Don't understand: " + target + " " + verb);
return whatever;
}

There is a lot more that could be said about the current coding style in use... I actually find it more or less impossible to read coherently, I can only guess at the number of bugs it is likely to contain. It should have been split long ago.
fae
fae
Agree with rip-off 100%. As it is the code is really difficult to read and understand. This particular issue with the NullPointerException was relatively easy to find - the trace had the exact line number and I could just look at the row and check which references can potentially cause the exception. But had it been e.g. a logic issue with some processing results..

As a next step for refactoring I'd even suggest that you move the command processing completely outside the "command switch" if you're going to have many different commands. I'd even go there with the current number of commands as that allows for easier testing and re-use of the said commands. This could be something like this (extending on rip-off's excellent example):



public class CommandSwitch {
/**
* Map containing all supported commands.
*/
private final java.util.Map<String, CommandProcessor> commandMap;
public CommandSwitch() {
commandMap = new java.util.Hashtable<String, CommandProcessor>();
}
/**
* Adds a new command to the switch. This can be called from whatever container/controller
* you're having.
*/
public void addCommandProcessor(String command, CommandProcessor processor) {
commandMap.put(command, processor);
}
public void logError(String error) {
System.out.println("ERROR - " + error);
}
public void printUsage(CommandSender sender, String usage) {
System.out.println("To " + sender + ": " + usage);
}
public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) {
if (!(sender instanceof Player)) {
logError("Invalid sender: " + sender);
return false;
}
Player player = (Player) sender;
String commandName = cmd.getName();
if (!commandName.equalsIgnoreCase("mc_web")) {
printUsage(player, "Unrecognised command: " + commandName);
return false;
}
if (args.length < 2) {
printUsage(player, "Not enough arguments: " + args.length);
return false;
}
String target = args[0];
String verb = args[1];
CommandProcessor processor = commandMap.get(target);
if(processor != null) {
return processor.processCommand(target, verb);
}
printUsage(player, "Don't understand: " + target + " " + verb);
return false;
}
public static void main(String[] args) {
CommandSwitch handler = new CommandSwitch();
handler.addCommandProcessor("money", new MoneyCommand());
handler.addCommandProcessor("item", new ItemCommand());
handler.addCommandProcessor("account", new AccountCommand());
CommandSender sender = new Player();
handler.onCommand(sender, new Command("mc_web"), "Money Label", new String[] { "money", "deposit" } );
handler.onCommand(sender, new Command("mc_web"), "Item Label", new String[] { "item", "deposit" } );
handler.onCommand(sender, new Command("mc_web"), "Unknown", new String[] { "invalid", "deposit" } );
}
/**
* These should be outside the class, now internal just to make the example standalone
*/
public static interface CommandProcessor {
public boolean processCommand(String name, String verb);
}
public static class MoneyCommand implements CommandProcessor {
public boolean processCommand(String name, String verb) {
System.out.println("MONEY: " + name + ", " + verb);
return true;
}
}
public static class ItemCommand implements CommandProcessor {
public boolean processCommand(String name, String verb) {
System.out.println("ITEM: " + name + ", " + verb);
return true;
}
}
public static class AccountCommand implements CommandProcessor {
public boolean processCommand(String name, String verb) {
System.out.println("ACCOUNT: " + name + ", " + verb);
return true;
}
}
public static class CommandSender {
/* whatever */
}
public static class Player extends CommandSender {
/* whatever */
}
public static class Command {
private final String name;
public Command(final String name) {
this.name = name;
}
public String getName() {
return name;
}
/* whatever */
}
}


The code is still lacking, it doesn't e.g. verify input values for nullness, but should give you a good idea of what I'm proposing. The design is actually according to "Command" design pattern, but I wouldn't recommend getting wild with different design patterns yet.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.