Enum to check margin

[MarWo22]

Hey guys,

I've been trying to make an enum which wel check which potion has the best margin to make. Buy my java skills aren't that great and i'm a little bit stuck. This is how far i came. But i cant seem to find a way to acces the potion with the highest margin. If anyone could help me that would be great!

 

Spoiler

import java.util.Optional;

public enum Potion {

	MARRENTILL,
	TARROMIN,
	HARRALANDER,
	RANARR,
	TOADFLAX,
	IRIT,
	AVANTOE,
	KWUARM,
	SNAPDRAGON,
	CADANTINE,
	LATADYME,
	DWARF_WEED;
	
	public final String name;
	public final String unfinishedName;
	public final Optional<String> herbPriceMax;
	public final Optional<String> herbPriceMin;
	public final Optional<String> unfinishedPriceMax;
	public final Optional<String> unfinishedPriceMin;
	public final int herbPrice;
	public final int unfinishedPrice;
	public final int margin;
	
	

	Potion() {
		
		name = name().substring(0,1).toUpperCase() + name().substring(1).toLowerCase() + name().replace("_", " ");
		unfinishedName = name + " potion (unf)";
		herbPriceMax = ItemLookup.get(name, Property.BUY_AVERAGE);
		herbPriceMin = ItemLookup.get(name, Property.SELL_AVERAGE);
		unfinishedPriceMax = ItemLookup.get(unfinishedName, Property.BUY_AVERAGE);
		unfinishedPriceMin = ItemLookup.get(unfinishedName, Property.SELL_AVERAGE);
		herbPrice = Math.max(Integer.valueOf(herbPriceMin.get()), Integer.valueOf(herbPriceMax.get()));
		unfinishedPrice = Math.min(Integer.valueOf(unfinishedPriceMin.get()), Integer.valueOf(unfinishedPriceMax.get()));
		margin = unfinishedPrice - herbPrice;
		
	}
	
}

 

The price lookup snippet i use returns the values in Optional<String> so thats why i first needed to transform them into integers

[Mysteryy]

Loop Potion.values() and find the one with the highest .margin

[MarWo22]

9 minutes ago, Mysteryy said:

Loop Potion.values() and find the one with the highest .margin

could you please write the code i cant seem to get it :]

[Eagle Scripts]

4 minutes ago, MarWo22 said:

could you please write the code i cant seem to get it :]

If you're unable to call the Enum#values function then I suggest learning the fundamentals of Java before trying to dive into scripting for OSBot .

[liverare]

Firstly:

name = name().substring(0,1).toUpperCase() + name().substring(1).toLowerCase() + name().replace("_", " ");
System.out.println(Potion.MARRENTILL.name); // MarrentillMARRENTILL

You're not generating the correct name. You should just hard code these values, because you're working with an enumerator. Enumerators are ideal for storing constants:

	MARRENTILL("Marrentill", "Marrentill potion (unf)")
	
	// ... 

	;
	
	private final String herbName;
	private final String unfinishedPotionName;

	Potion(String herbName, String unfinishedPotionName) {
		
		this.herbName = herbName;
		this.unfinishedPotionName = unfinishedPotionName;
		
		// ...

Then, you're going to need to look at what it is ItemLookup#get is actually returning, because looking at your variables:

public final Optional<String> herbPriceMax;
public final Optional<String> herbPriceMin;
public final Optional<String> unfinishedPriceMax;
public final Optional<String> unfinishedPriceMin;

It's returning an Optional<String> which is wrapping the value. You need to retrieve the value like so:

herbPriceMax = ItemLookup.get(name, Property.BUY_AVERAGE).orElse(null);

This will attempt to get the value stored in the optional, or else it will return null. However, from looking at the <String>, the value will be a string that needs to be parsed to a Long:

String herbPriceMaxStr = ItemLookup.get(name, Property.BUY_AVERAGE).orElse(null);
long herbPriceMax = (herbPriceMaxStr != null ? Long.parseLong(herbPriceMaxStr) : 0);

Once you've done that, you may wish to consider not generating these values on the constructor. Why? Because margins change and you'd want to know about that in real time. You're going to want to build getter methods that'll retrieve the latest prices:

/**
 * greater than 1 = priced
 *              0 = not priced
 *             -1 = failed to retrieve/cast price
 */
public long getMaxHerbPrice() {
	
	long price = 0L;
	
	try {
	
		price = ItemLookup.get(name, Property.BUY_AVERAGE).map(Long::parseLong).get();
	
	} catch (Exception e) {
	
		price = -1L;
	}

	return price;
}

(this was rushed so you can do a better job than I've done).

This should enable you to do:

long marrentillMaxHerbPrice = Potion.MARRENTILL.getMaxHerbPrice();

However, you're going to want to setup some sort of caching mechanism to avoid grabbing new prices each time. unless it's absolutely critical.

 

:EDIT:

Whenever you're working with prices and quantities, do so in LONG, not INTEGER. Why? Because an integers are 32-bit and their min/max values are only -2,147,483,648  and 2,147,483,647.

https://stackoverflow.com/questions/15004944/max-value-of-integer

Edited November 19, 2018 by liverare

[Mysteryy]

@liverare Good explanation but 2 comments:

1) It might not always be a good idea to query live prices in the getter. Instead you might add a throttle where it only updates max every few minutes, else returns the cached value. Otherwise you could accidentally spam requests. Since it seems like hes just starting to get into it, I can see this as likely to happen.

2) There's no point to use long. Rs uses 32 bit signed int, so prices will never go over (2^31) - 1

 

 

Edit: just saw your comment about adding a caching mechanism to not grab prices everytime, but the code didnt have it.

Edited November 19, 2018 by Mysteryy

[liverare]

5 minutes ago, Mysteryy said:

@liverare Good explanation but 2 comments:

1) It might not always be a good idea to query live prices in the getter. Instead you might add a throttle where it only updates max every few minutes, else returns the cached value. Otherwise you could accidentally spam requests. Since it seems like hes just starting to get into it, I can see this as likely to happen.

2) There's no point to use long. Rs uses 32 bit signed int, so prices will never go over (2^31) - 1

 

 

Edit: just saw your comment about adding a caching mechanism to not grab prices everytime, but the code didnt have it.

1. Caching prices is definitely good. However, that's a layer of logic that should be handled outside of the raw routine that grabs those values. If you start to merge your caching code with your price retrieval code, it'll get unwieldy. Also, the user case might be that the prices are so volatile and necessary that caching isn't an option.
2. Wrong. Players have had multiple max cash stacks and there are items that are worth billions. Now, it's unlikely the script would ever work with those sorts of figures. However, your code should be ready to handle situations where it could. For instance, what if somebody using the script needs to calculate a huge total buy/sell? If the script uses integers, it could overflow. You can (and so should) account for this sooner rather than later.

[Mysteryy]

1 hour ago, liverare said:

1. Caching prices is definitely good. However, that's a layer of logic that should be handled outside of the raw routine that grabs those values. If you start to merge your caching code with your price retrieval code, it'll get unwieldy. Also, the user case might be that the prices are so volatile and necessary that caching isn't an option.
2. Wrong. Players have had multiple max cash stacks and there are items that are worth billions. Now, it's unlikely the script would ever work with those sorts of figures. However, your code should be ready to handle situations where it could. For instance, what if somebody using the script needs to calculate a huge total buy/sell? If the script uses integers, it could overflow. You can (and so should) account for this sooner rather than later.

1. Yea, like I said I missed your comment. But that's why i said every few minutes. Refresh it as often as the use case requires. 

2. No, I think you're misunderstanding his use case. He's retrieving the price, from the GE, for a single item. This value can never and will never go over max int, because the price for any single item can never go over max int. So no, in this case, its absolutely pointless to use long to retrieve the price from the GE for a single item (which is indeed his use case). If he was calculating the total value for a stack, then yes it makes sense, but hes not. 

[liverare]

1 hour ago, Mysteryy said:

1. Yea, like I said I missed your comment. But that's why i said every few minutes. Refresh it as often as the use case requires. 

2. No, I think you're misunderstanding his use case. He's retrieving the price, from the GE, for a single item. This value can never and will never go over max int, because the price for any single item can never go over max int. So no, in this case, its absolutely pointless to use long to retrieve the price from the GE for a single item (which is indeed his use case). If he was calculating the total value for a stack, then yes it makes sense, but hes not. 

Fair enough. I'd still use long values just because it can be reused elsewhere without having to worry about this issue.