Optymalizacja kodu

0

Witajcie ;)
Czy nudzi sie moze komuś z was na tyle żeby przejrzeć ten kod i powiedzieć co można zrobić lepiej? Jest to prosty plugin do gry.

package pl.danek;

import java.io.File;
import java.util.ArrayList;

import org.bukkit.Bukkit;
import org.bukkit.ChatColor;
import org.bukkit.Material;
import org.bukkit.command.Command;
import org.bukkit.command.CommandSender;
import org.bukkit.configuration.file.FileConfiguration;
import org.bukkit.entity.Player;
import org.bukkit.inventory.ItemStack;
import org.bukkit.plugin.PluginManager;
import org.bukkit.plugin.java.JavaPlugin;

public class DropStone extends JavaPlugin {
	
	private BlockBreakEventListener listener;
	

	private ArrayList<String> players = new ArrayList<>();
	
	private ArrayList<OreBlock> blocks;
	
	private String em = "Drop.Stone.EMERALD_ORE";
	private String dm = "Drop.Stone.DIAMOND_ORE";
	private String rs = "Drop.Stone.REDSTONE_ORE";
	private String gl = "Drop.Stone.GOLD_ORE";
	private String ir = "Drop.Stone.IRON_ORE";
	private String co = "Drop.Stone.COAL_ORE";
	
	@Override
	public void onEnable() {
		PluginManager pm = getServer().getPluginManager();
		FileConfiguration c = this.getConfig();
		if(!new File(getDataFolder().toString()).exists()) {
			generateDefaultConfig(c);
			
			this.saveConfig();
		}
		blocks = generateBlockList(c);
		
		listener = new BlockBreakEventListener(blocks, this);
		pm.registerEvents(this.listener, this);
	}
	
	@Override
	public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args){
		if (cmd.getName().equalsIgnoreCase("dropstone") && args.length==1) {
			switch (args[0]) {
				case "p":
					for (OreBlock o : blocks) {
						sender.sendMessage(o.getDropItem()+":"+(double) o.getChance()/10+" %");
					}
					break;
				case "on":
					if(!players.contains(sender)){
						players.add(sender.getName());
					}
					break;
				case "off":
					if(players.contains(sender)){
						players.remove(sender.getName());
					}
					break;
				default:
				case "help":
					sender.sendMessage(ChatColor.YELLOW+"Komendy pluginu DropStone");
					sender.sendMessage("/dropstone p - wyswietla szanse na mineraly");
					sender.sendMessage("/dropstone off/on - wylacza/wlacza powiadomienia");
					break;
			}
		}
		return true;
	}
	
	private ArrayList<OreBlock> generateBlockList(FileConfiguration c) {
		OreBlock emerald = new OreBlock(c.getInt(em + ".maxY"), c.getInt(em + ".chance"), c.getString(em + ".message"), c.getItemStack(em + ".ItemStack"),c.getInt(em + ".exp"));
		OreBlock diamond = new OreBlock(c.getInt(dm + ".maxY"), c.getInt(dm + ".chance"), c.getString(dm + ".message"), c.getItemStack(dm + ".ItemStack"),c.getInt(em + ".exp"));
		OreBlock redston = new OreBlock(c.getInt(rs + ".maxY"), c.getInt(rs + ".chance"), c.getString(rs + ".message"), c.getItemStack(rs + ".ItemStack"),c.getInt(em + ".exp"));
		OreBlock goldore = new OreBlock(c.getInt(gl + ".maxY"), c.getInt(gl + ".chance"), c.getString(gl + ".message"), c.getItemStack(gl + ".ItemStack"),c.getInt(em + ".exp"));
		OreBlock ironore = new OreBlock(c.getInt(ir + ".maxY"), c.getInt(ir + ".chance"), c.getString(ir + ".message"), c.getItemStack(ir + ".ItemStack"),c.getInt(em + ".exp"));
		OreBlock coalore = new OreBlock(c.getInt(co + ".maxY"), c.getInt(co + ".chance"), c.getString(co + ".message"), c.getItemStack(co + ".ItemStack"),c.getInt(em + ".exp"));
		ArrayList<OreBlock> blocks = new ArrayList<OreBlock>();
		blocks.add(emerald);
		blocks.add(diamond);
		blocks.add(redston);
		blocks.add(goldore);
		blocks.add(ironore);
		blocks.add(coalore);
		return blocks;
	}

	private void generateDefaultConfig(FileConfiguration c) {
		this.saveDefaultConfig();
		
		c.createSection(em);
		c.set(em + ".maxY", 20);
		c.set(em + ".chance", 1);
		c.set(em + ".exp", 100);
		c.set(em + ".message", "znalazl szmaragd!");
		c.set(em + ".ItemStack", new ItemStack(Material.EMERALD));
		
		c.createSection(dm);
		c.set(dm + ".maxY", 10);
		c.set(dm + ".chance", 4);
		c.set(em + ".exp", 80);
		c.set(dm + ".message", "znalazl diameny!");
		c.set(dm + ".ItemStack", new ItemStack(Material.DIAMOND));
		
		c.createSection(rs);
		c.set(rs + ".maxY", 30);
		c.set(rs + ".chance", 30);
		c.set(em + ".exp", 80);
		c.set(rs + ".message", "znalazl redstona!");
		c.set(rs + ".ItemStack", new ItemStack(Material.REDSTONE));
		
		c.createSection(gl);
		c.set(gl + ".maxY", 20);
		c.set(gl + ".chance", 10);
		c.set(em + ".exp", 80);
		c.set(gl + ".message", "znalazl zloto!");
		c.set(gl + ".ItemStack", new ItemStack(Material.GOLD_ORE));
		
		c.createSection(ir);
		c.set(ir + ".maxY", 35);
		c.set(ir + ".chance", 15);
		c.set(em + ".exp", 80);
		c.set(ir + ".message", "znalazl zelazo!");
		c.set(ir + ".ItemStack", new ItemStack(Material.IRON_ORE));
		
		c.createSection(co);
		c.set(co + ".maxY", 50);
		c.set(co + ".chance", 30);
		c.set(em + ".exp", 50);
		c.set(co + ".message", "znalazl redstona");
		c.set(co + ".ItemStack", new ItemStack(Material.COAL));
	}
	
	@Override
	public void onDisable() {
		Bukkit.getServer().broadcastMessage(ChatColor.RED + "DropStone disabled!");
	}
	public ArrayList<String> getPlayers() {
		return players;
	}
	
}
 
package pl.danek;


import java.util.ArrayList;
import java.util.Random;

import org.bukkit.Bukkit;
import org.bukkit.ChatColor;
import org.bukkit.Material;
import org.bukkit.World;
import org.bukkit.block.Block;
import org.bukkit.entity.Player;
import org.bukkit.event.EventHandler;
import org.bukkit.event.Listener;
import org.bukkit.event.block.BlockBreakEvent;

public class BlockBreakEventListener implements Listener {

	private Random r = new Random();
	private ArrayList<OreBlock> oreBlocks;
	private DropStone plugin;
	
	
	public BlockBreakEventListener(ArrayList<OreBlock> oreBlocks, DropStone plugin) {
		this.oreBlocks = oreBlocks;
		this.plugin=plugin;
	}




	@EventHandler
	public void onBlockBreakEvent(BlockBreakEvent e){
		Block block = e.getBlock();
		
		if(block.getType().equals(Material.GOLD_ORE)||
			block.getType().equals(Material.IRON_ORE)||
			block.getType().equals(Material.COAL_ORE)||
			block.getType().equals(Material.DIAMOND_ORE)||
			block.getType().equals(Material.REDSTONE_ORE)||
			block.getType().equals(Material.EMERALD_ORE)){

			e.setCancelled(true);
			e.getBlock().setType(Material.AIR);
		}

		if (block.getType().equals(Material.STONE)) {
			for (OreBlock oreBlock : oreBlocks) {
				if (block.getY()<=oreBlock.getMaxY()&&r.nextInt(999)+1<=oreBlock.getChance()) {
					Player player = e.getPlayer();
					
					player.getInventory().addItem(oreBlock.getDropItem());
					player.giveExp(oreBlock.getExp());
					
					printInfo(e, oreBlock);
				}
			}
		}
	}




	private void printInfo(BlockBreakEvent e, OreBlock oreBlock) {
		for (Player p : Bukkit.getOnlinePlayers()) {
			if(plugin.getPlayers().contains(p.getName())){
				p.sendMessage(ChatColor.DARK_AQUA+"Gracz "+e.getPlayer().getDisplayName()+ChatColor.DARK_AQUA+oreBlock.getMessage());
			}
		}
	}

}
 
 package pl.danek;

import org.bukkit.inventory.ItemStack;


public class OreBlock {
	
	private int maxY;
	private int chance;
	private String message;
	private ItemStack dropItem;
	private int exp;
	
	public OreBlock(int maxY, int chance, String message, ItemStack dropItem, int exp) {
		this.maxY = maxY;
		this.chance = chance;
		this.message = message;
		this.dropItem = dropItem;
		this.exp = exp;
	}

	public int getMaxY() {
		return maxY;
	}

	public int getChance() {
		return chance;
	}

	public String getMessage() {
		return message;
	}

	public ItemStack getDropItem() {
		return dropItem;
	}

	public int getExp() {
		return exp;
	}
	
	
	
}
0
OreBlock emerald = new OreBlock(c.getInt(em + ".maxY"), c.getInt(em + ".chance"), c.getString(em + ".message"), c.getItemStack(em + ".ItemStack"),c.getInt(em + ".exp"));
        OreBlock diamond = new OreBlock(c.getInt(dm + ".maxY"), c.getInt(dm + ".chance"), c.getString(dm + ".message"), c.getItemStack(dm + ".ItemStack"),c.getInt(em + ".exp"));
        OreBlock redston = new OreBlock(c.getInt(rs + ".maxY"), c.getInt(rs + ".chance"), c.getString(rs + ".message"), c.getItemStack(rs + ".ItemStack"),c.getInt(em + ".exp"));
        OreBlock goldore = new OreBlock(c.getInt(gl + ".maxY"), c.getInt(gl + ".chance"), c.getString(gl + ".message"), c.getItemStack(gl + ".ItemStack"),c.getInt(em + ".exp"));
        OreBlock ironore = new OreBlock(c.getInt(ir + ".maxY"), c.getInt(ir + ".chance"), c.getString(ir + ".message"), c.getItemStack(ir + ".ItemStack"),c.getInt(em + ".exp"));
        OreBlock coalore = new OreBlock(c.getInt(co + ".maxY"), c.getInt(co + ".chance"), c.getString(co + ".message"), c.getItemStack(co + ".ItemStack"),c.getInt(em + ".exp"));
        ArrayList<OreBlock> blocks = new ArrayList<OreBlock>();
        blocks.add(emerald);
        blocks.add(diamond);
        blocks.add(redston);
        blocks.add(goldore);
        blocks.add(ironore);
        blocks.add(coalore);

:|

1
    switch (args[0]) {
                case "p":
                    for (OreBlock o : blocks) {
                        sender.sendMessage(o.getDropItem()+":"+(double) o.getChance()/10+" %");
                    }
                    break;
                case "on":
                    if(!players.contains(sender)){
                        players.add(sender.getName());
                    }
                    break;
                case "off":
                    if(players.contains(sender)){
                        players.remove(sender.getName());
                    }
                    break;
                default:
                case "help":
                    sender.sendMessage(ChatColor.YELLOW+"Komendy pluginu DropStone");
                    sender.sendMessage("/dropstone p - wyswietla szanse na mineraly");
                    sender.sendMessage("/dropstone off/on - wylacza/wlacza powiadomienia");
                    break;
            }
        }

niech opcje będą kluczami w mapie, a wartościami będą obiekty reprezentujące akcje.

0

@Koziołek
czy może to tak wyglądać? ;) tylko pytanie mam czy to wysyłanie tych argumentów metod wyglada ładnie i czy nie da sie tego zrobić lepiej? ;)

 
package pl.danek;

import java.util.HashMap;
import java.util.Map;

import org.bukkit.command.Command;
import org.bukkit.command.CommandSender;

public class CommandManager {

	
	Map<String, CommandAction> commands = new HashMap<String, CommandAction>();
	
	public void useCommand(CommandSender sender, Command cmd, String label, String[] args){
		commands.get(cmd.getName()).action(sender, cmd, label, args);
	}
	
	public void addCommand(String command,CommandAction action){
		commands.put(command, action);
	}
}



package pl.danek;

import org.bukkit.command.Command;
import org.bukkit.command.CommandSender;

public interface CommandAction {

	public void action(CommandSender sender, Command cmd, String label, String[] args);
	
}

1

W kwestii nazewniczej. Zamiast addCommand użyj register ponieważ nie tyle co dodajesz komendę, co ją rejestrujesz w systemie. Zamiast useCommand zrób po prostu call.

W dodatku masz tu pewną duplikację w postaci CommandAction i Command. Ta druga kalsa wygląda tak jakby przechowywała tylko nazwę i w switch była ona wykorzystana by odpalić pewną logikę. Logikę umieściłeś w implementacji CommandAction co powoduje, ze Command nadal nie daje nic poza nazwą.

0

tak czytam sobie o tym budowniczym i we wszystkich przykładach (np. w tym co podałem w komentarzu wyżyj) wszystkie tworzone obiekty mają z góry ustalone wartości. A jak zrobić jeśli wartości tych pół wczytuje z pliku?

W przykładzie znalazłem takie coś:

 
/* nasz glowny interface */
abstract class Builder {
    protected ZestawKomputerowy zestawKomputerowy;
 
    public void newZestaw(){
        zestawKomputerowy = new ZestawKomputerowy();
    }
 
    public ZestawKomputerowy getZestaw(){
        return zestawKomputerowy;
    }
 
    public abstract void buildMonitor();
    public abstract void buildProcesor();
    public abstract void buildGrafika();
    public abstract void buildRam();
    public abstract void buildHdd();
}
 
 
class ZestawXT001 extends Builder {
 
 
    public void buildMonitor(){
        zestawKomputerowy.setMonitor("Benq 19");
    }
 
    public void buildProcesor(){
        zestawKomputerowy.setProcesor("amd");
    }
 
    public void buildGrafika(){
        zestawKomputerowy.setGrafika("ATI");
    }
 
    public void buildRam(){
        zestawKomputerowy.setRam("DDR3");
    }
 
    public void buildHdd(){
 
        Scanner in = new Scanner(System.in);
 
        int t;
        while(true){
            System.out.println("Dysk do wyboru: (1) Samsung, (2) Segate, (3) Caviar");
            t = in.nextInt();
            if(t>0 && t<4) break;
        }
 
    String wynik="";
    if(t==1) wynik = "Samsung";
    else if(t==2) wynik = "Segate";
    else if(t==3) wynik = "Caviar";
 
    zestawKomputerowy.setHdd(wynik);
 
    }
}

i teraz właśnie moje pytanie jak to zrobić zeby te wartości wczytać z pliku? Wybaczcie, pewnie dla was, głupie pytania ale dopiero sie ucze ;)

1 użytkowników online, w tym zalogowanych: 0, gości: 1