[Critiques Welcome] Multiple Banking Snippets

[ikk]

Decided I would share my solutions for Banking to help beginners as well as to receive critiques to improve my code.

 

Firstly, I typically do two things before I start banking. I generate a list of items that I DON'T want to be deposited (Banking Exceptions) & a list of items that my character will need to withdraw from the bank (Needed Supplies).

 

Here is my method to generate the Deposit Exceptions:

	public LinkedList<String> getDepositExceptions() {
		LinkedList<String> neededItems = new LinkedList<String>();
		if (Config.enableAttPot){
			neededItems.add("Attack potion(4)");
		}
		if (Config.enableStrPot){
			neededItems.add("Strength potion(4)");
		}
		if (Config.enableSupAttPot){
			neededItems.add("Super attack(4)");
		}
		if (Config.enableSupStrPot){
			neededItems.add("Super strength(4)");
		}
		if (Config.enableCombatPot){
			neededItems.add("Combat potion(4)");
		}
		neededItems.add("Lobster");
		return neededItems;
	}

Explained:

 

So I'm creating a list of items which I do not want to deposit into my bank. This list will be used later when I want to deposit all of the items in my inventory (except for those found in this list).

I'm using if statements for some items because the items may not be relevant for all users. This is handy if you have a GUI for your script where not everyone will have the same banking exceptions. Then for items which will be universal for your script (in this example, Lobster) you can simply add them to the list.

 

My method to Deposit All items (with the exception of those found in the getDepositExceptions() method above):

	public void depositUnwanted() throws InterruptedException{
		for (Item i : S.getInventory().getItems()) {
			if (i != null && !getDepositExceptions().contains(i.getName())) {
				S.log("Banking: " + i.getName());
				i.interact("Deposit-All");
				Script.sleep(Script.random(350,500));
			}
		}
	}

Explained:

 

This will simply create a for loop which will look through all the items found in your inventory. If the item isn't an item found in the list generated by getDepositExceptions, it will deposit all of that item. May be beneficial to use a conditional sleep after the deposit instead of my way.

 

My method to generate a list of Needed Supplies:

	public Entry<String, Integer> getNeededSupplies() {
		LinkedHashMap<String, Integer> neededItems = new LinkedHashMap<String, Integer>();
		
		if (Config.enableAttPot && (!S.inventory.contains("Attack potion(4)") || (S.getInventory().getAmount("Attack potion(4)") < Config.attAmt) )){
			neededItems.put(Constants.ATTACK_B[0], (Config.attAmt - (int) S.getInventory().getAmount("Attack potion(4)")));
		}
		if (Config.enableStrPot && (!S.inventory.contains("Strength potion(4)") || (S.getInventory().getAmount("Strength potion(4)") < Config.strAmt))){
			neededItems.put(Constants.STRENGTH_B[0], (Config.strAmt - (int) S.getInventory().getAmount("Strength potion(4)")));
		}
		if (Config.enableSupAttPot && (!S.inventory.contains("Super attack(4)") || (S.getInventory().getAmount("Super attack(4)") < Config.supAttAmt))){
			neededItems.put(Constants.SUPER_ATTACK_B[0], (Config.supAttAmt - (int) S.getInventory().getAmount("Super attack(4)")));
		}
		if (Config.enableSupStrPot && (!S.inventory.contains("Super strength(4)") || (S.getInventory().getAmount("Super strength(4)") < Config.supStrAmt))){
			neededItems.put(Constants.SUPER_STRENGTH_B[0], (Config.supStrAmt - (int) S.getInventory().getAmount("Super strength(4)")));
		}
		if (Config.enableCombatPot && (!S.inventory.contains("Combat potion(4)") || (S.getInventory().getAmount("Combat potion(4)") < Config.combatAmt))){
			neededItems.put(Constants.COMBAT_B[0], (Config.combatAmt - (int) S.getInventory().getAmount("Combat potion(4)")));
		}
		if (S.getInventory().getAmount("Lobster") < Config.foodAmt){
			neededItems.put(Config.foodName, (Config.foodAmt - (int) S.getInventory().getAmount("Lobster")));
		}
		
		final Set<Entry<String, Integer>> values = neededItems.entrySet();
		final int maplength = values.size();
		final Entry<String, Integer>[] test = new Entry[maplength];
		values.toArray(test);
		if (test.length > 0){
			return test[0];
		}
		else return null;
	}

Explained:

 

So here I am creating a Linked Hash Map (From my understanding, this is similar to a List). I've done this so that I can store the Item name & the amount that should be withdrawn in the same grouping to be used for later. This time, it is best to use an if statement for EVERY item because we need to check if your inventory doesn't already contain the item.

We're also doing some math to determine the correct amount to withdraw by subtracting the current amount in inventory from the maximum amount you should have. For me, I store the maximum in a Config class which grabs the data from my GUI (IE. config.attAmt)

 

My method for withdrawing item(s):

	private void withdraw(String itemName, int amt) throws InterruptedException {
		Item item = this.S.bank.getItem(itemName);
		if (S.getBank().contains(itemName)) {

			S.getBank().withdraw(item.getName(), amt);
			Script.sleep(Script.random(350, 600));

		} else {

			S.log("Ran out of " + itemName + ", stopping.");
			S.stop();
		}
	}

Explained:

 

A simple method with 2 parameters, the name of the item, and the amount to be withdrawn. If the bank contains your item, it will withdraw the amount given. If the bank does not contain your item, it will print into the Logger that you have run out of the item name, and will end your script. Again, it may be useful to add a conditional sleep instead of this random integer sleep.

 

My method to open the nearest bank:

	private void openBank() throws InterruptedException {

		S.getBank().open();
		new ConditionalSleep(5000) {

			@Override
			public boolean condition() throws InterruptedException {
				return S.getBank().isOpen();
			}
		}.sleep();
		S.log("Banking");
	}

Explained:

 

Will simply open the nearest bank, and have a 5-second conditional sleep which will wait 5 seconds if the bank is not open, or will cut the sleep off short when it sees that the bank is, in fact, open.

 

Putting it all together: 

	if (S.getBank() != null) {
		if (!S.getBank().isOpen())
			openBank();
		else {
			//Deposits all items except bank exceptions
			for (Item i : S.getInventory().getItems()) {
				if (i != null && !getDepositExceptions().contains(i.getName())) {
					S.log("Banking: " + i.getName());
					i.interact("Deposit-All");
					Script.sleep(Script.random(350,500));
				}
			}
			if (getNeededSupplies() != null){
				S.log("Need to withdraw: " + getNeededSupplies().getKey() + ", " + getNeededSupplies().getValue() );
				//.getKey() will return our LinkedHashMap String / itemName 
				//& .getValue() will return our Integer / Amount to withdraw
				withdraw(getNeededSupplies().getKey(), getNeededSupplies().getValue());
			}
		}
	}

Explained:

 

This is essentially a fully working Banking Class now. It will open the nearest bank if it's not already open. Then it will deposit all the items found in the inventory which aren't needed / desired. Then it will withdraw all of the items / supplies which will be needed for the task.

 

Hopefully, this is useful to you guys. I'm looking to improve my knowledge as well so if you see anything in this thread that can be optimized / improved, I would love to hear it!

Edited February 23, 2016 by ikk

[Botre]

I have to say your code is very readable and the presentation of your question makes it fun to answer it.

 

A few tips:

Config.enableCombatPot)

Avoid using static variables to store configuration or instance-specific state values, when running multiple instances of the same script the last launched script's values will override the values of all older instances. Create a configuration file for each script instance.

 

http://stackoverflow.com/questions/413898/what-does-the-static-keyword-do-in-a-class

getDepositExceptions().contains(i.getName())

A small tip: "contains" tends to be less efficient than "starts with" or even "equals".

 

https://docs.oracle.com/javase/tutorial/java/data/comparestrings.html

i.interact("Deposit-All");
Script.sleep(Script.random(350,500));

Sadly enough, interactions do fail sometimes, you sleep should be conditioned by the success of the triggering interaction.

if(interact()) { sleep(); }

Not a big deal though.

getNeededSupplies()

This method contains the least elegant code of the snippet, it would greatly benefit from some struct magic.

... Init

Supply lobster = new Supply("Lobster", 15);
Supply attackPotion = new Supply("Attack potion(3)", 1);
List<Supply> supplies = new ArrayList();

... Feed list

supplies.add(lobster);
supplies.add(attackPotion);

... In bank

for each(Supply s; supplies) if(s.shouldRestock()) getBank().withdraw(s.getName(), s.calculateRestockValue());

Your map solution works, but there's so much duplicate code that it could become hard to maintain after a while.

[Patyfatycake]

This logic could be extrapolated to another method to make it easier to read.

Config.enableStrPot && (!S.inventory.contains("Strength potion(4)") || (S.getInventory().getAmount("Strength potion(4)") < Config.strAmt))

shouldWithdrawItem(boolean enabled, String itemName, int amount)

if (test.length > 0){
   return test[0];
}
else return null;

return test.length > 0 ? test[0]] : null;

final Set<Entry<String, Integer>> values = neededItems.entrySet();
final int maplength = values.size();
final Entry<String, Integer>[] test = new Entry[maplength];

Looks pretty ugly ngl.

[Bobrocket]

public LinkedList<String> getDepositExceptions() {
		LinkedList<String> neededItems = new LinkedList<String>();
		if (Config.enableAttPot){
			neededItems.add("Attack potion(4)");
		}
		if (Config.enableStrPot){
			neededItems.add("Strength potion(4)");
		}
		if (Config.enableSupAttPot){
			neededItems.add("Super attack(4)");
		}
		if (Config.enableSupStrPot){
			neededItems.add("Super strength(4)");
		}
		if (Config.enableCombatPot){
			neededItems.add("Combat potion(4)");
		}
		neededItems.add("Lobster");
		return neededItems;
	}

This can be optimised in many ways. Running this is going to be more expensive than something like this:

public enum Supplies {
    LOBSTER("Lobster", 15),
    ATTACK_POT("Attack potion(4)", 1),
    ;

    private String name;
    private int amt;

    Supplies(String name, int amt) {
        this.name = name;
        this.amt = amt;
    }

    public int getAmount() { return amt; }
    public String getName() { return name; }
}

//on start

private List<Supplies> supplyList = new ArrayList<>();

if (Config.isAttackEnabled) supplyList.add(Supplies.ATTACK_POT);
//...

Then you can do something like:

//bank
for (Item item : getInventory().getItems()) {
    for (Supplies supply : supplyList) if (!supply.getName().equalsIgnoreCase(item.getName())) item.interact("Deposit-All");
}

You could have a functional interface in your enum and have a static populate(Script) method, kinda looking like:

public interface Functional<T> { public boolean perform(T param); }

//enum
LOBSTER("Lobster", 15, (script) -> true),
ATTACK_POT("Attack potion(4)", 1, (script) -> Config.isAttackEnabled),
COINS("Coins", 10, (script) -> script.getInventory().getAmount("Coins") < 10), //al kharid gate?
;

private String name;
private int amt;
private Functional<Script> condition;

public Functional<Script> getCondition() { return condition; }

public static List<Supplies> populate(Script instance) {
    List<Supplies> vals = new ArrayList<>();
    for (Supplies supply : values()) if (supply.getCondition().perform(instance)) vals.add(supply);
    return vals;
}

List<Supplies> supplyList = Supplies.populate(this);

Obviously, running these conditions at the start would mean that they wouldn't run again (unless you did populate(Script) again), but it's a nice little way to try and keep things a little organised.

[Kozs]

Pretty verbose, but that's java. :P

[Team Cape]

	public LinkedList<String> getDepositExceptions() {
		LinkedList<String> neededItems = new LinkedList<String>();
		if (Config.enableAttPot){
			neededItems.add("Attack potion(4)");
		}
		if (Config.enableStrPot){
			neededItems.add("Strength potion(4)");
		}
		if (Config.enableSupAttPot){
			neededItems.add("Super attack(4)");
		}
		if (Config.enableSupStrPot){
			neededItems.add("Super strength(4)");
		}
		if (Config.enableCombatPot){
			neededItems.add("Combat potion(4)");
		}
		neededItems.add("Lobster");
		return neededItems;
	}

	public void depositUnwanted() throws InterruptedException{
		for (Item i : S.getInventory().getItems()) {
			if (i != null && !getDepositExceptions().contains(i.getName())) {
				S.log("Banking: " + i.getName());
				i.interact("Deposit-All");
				Script.sleep(Script.random(350,500));
			}
		}
	}

Only looked at the first couple, but here are my thoughts on improving those two:

public boolean isAnException(Item x) {
    String name = x.getName();
    return name.contains("Attack pot") || name.contains("Strength pot") || name.contains(“Super att”) || name.contains(“Super str”) || 
name.contains(“Combat pot”) || name.contains(“Lobster”);
}

public void depositUnwanted() throws InterruptedException {
	for(Item i: inventory.getItems()) {
		if(!isAnException(i)) {
			i.interact(“Deposit-All”);
			sleep(random(350, 500));
		}
	}
}

Edit: Wow, I feel stupid. Just realized this thread is like 5 months old lol... #gravedigger

Edited July 15, 2016 by Imateamcape

[Solzhenitsyn]

Hi, let me tell you quickly the difference between hash table, linked list, and array data structure.

 

LL: Non-contiguous in memory, each node has data item and pointer to next. Good performance for iterate, O(n) performance for specific look up, O(n) for search. 

 

Array: Contiguous in memory, no node just data item. Good performance for iterate, O(1) performance for specific look up, O(n) for search.

 

Hash table: Uses hash function to map a key (input) to a location (data), can't iterate but O(1) performance for specific look up and more importantly O(1) performance for searching. No key = can't search.

 

Edit: so i think its not appropriate to use hash table here. but good useful code anyways thanks for that.

 

Edit 2: usually you should use array because it is more efficient for memory. the only problem with array vs linkedlist is that insert a new element into the middle of array is slow, because you have to move all the shit in front of it. linkedlist you just need to add new node in memory, copy pointer of n-1 node into your new node and location of your new node into n-1 ptr. in java you dont get to use pointers though so idk, sry cant help there.

 

So here I am creating a Linked Hash Map (From my understanding, this is similar to a List). I've done this so that I can store the Item name & the amount that should be withdrawn in the same grouping to be used for later. This time, it is best to use an if statement for EVERY item because we need to check if your inventory doesn't already contain the item.   

Edited July 17, 2016 by FuckingAshole