Ahh this really isn't great enumerations are a strong way to give constants a bit more flavor context but that is really where it should end your packing so much stuff in your enum class is defeating the purpose of objective based programing. The point is to make things clean and simple without over engineering it, Java is already verbose enough think lazy when you create things it truly is the best practice.
You have multiple methods doing the exact same check it can be condensed into one.
public static void fillSetsOnStartup() {
int counter = 0;
for (SlayerTasks task : SlayerTasks.values()) {
counter++;
if (task.getTaskMaster() == SlayerMaster.VANNAKA)
EASY_TASKS.add(task);
else if (task.getTaskMaster() == SlayerMaster.DURADEL)
MEDIUM_TASKS.add(task);
else if (task.getTaskMaster() == SlayerMaster.KURADEL)
HARD_TASKS.add(task);
else if (task.getTaskMaster() == SlayerMaster.SUMONA)
ELITE_TASKS.add(task);
else if (task.getTaskMaster() == SlayerMaster.BRAVEK)
EXTREME_TASKS.add(task);
System.out.println("Successfully added " + counter + " Slayer tasks");
}
}
If data already exists don't duplicate it also this method and its call isn't needed to populate static collections you can simply call a static block
static {
}
while ((!meetsTaskKC(player, selectedTask)
|| !meetsSlayerReq(player, selectedTask)
|| repeatTask(player, selectedTask))) {
switch (currentSlayerMaster) {
case VANNAKA:
selectedTask = (SlayerTasks) Misc.randomFromList(EASY_TASKS);
break;
case DURADEL:
selectedTask = (SlayerTasks) Misc.randomFromList(MEDIUM_TASKS);
break;
case KURADEL:
selectedTask = (SlayerTasks) Misc.randomFromList(HARD_TASKS);
break;
case SUMONA:
selectedTask = (SlayerTasks) Misc.randomFromList(ELITE_TASKS);
break;
case BRAVEK:
selectedTask = (SlayerTasks) Misc.randomFromList(EXTREME_TASKS);
break;
}
System.out.println("SELECTED TASK = " + selectedTask.name());
}
Here you are using a while loop creating an imposed recursive iteration check, we have had Streams since java 8 should check into them you can filter out your data sets so they only contain what is currently usable.
public static SlayerTasks forId(int id) {
for (SlayerTasks tasks : SlayerTasks.values()) {
if (tasks.ordinal() == id) {
return tasks;
}
}
return null;
}
This is opening up a chance for a null pointer exception look into Optionals also don't use ordinal() it value is exclusively order specific and any changes to the constants stack will break what is returned.
SlayerTasks selectedTask = null;
//this as well can give a null pointer exception. Optionals
Optional<SlayerTask> o = Optional.empty();
o.isPresent();
o.isEmpty();
//optionals add a great deal of functional programming to java to make sure we don't need to use null which or and else functions.
