Trying to make Banking class, Errors

[slazter]

Okay so i've been trying to learn more about overloading constructors, the "this" keyword, and to actually initialize values in constructors.
I figured since many of my scripts contain some banking elements, why not try to make a Banking class, where i initialize what i want to withdraw, and amount in the constructor, to practice and make it stick better.
However i ran into some major troubles, I keep getting nullPointerException whenever i try to create an object and initialize the values in my script, and it won't even work even in the banking class itself. Im pretty clueless on what to do, so any help would be greatly appreciated. 

I'll post the code, and yes atm it's far from great, but i can't really improve it unless i even get it to run, so i'll just post here and any help or fingers pointing me in the right direction would be greatly appreciated.

also, the object was supposed to be place inside a method, to only have the object available when i want and not during entire script duration when i don't need it, but for testing purposes i thought i could stick it in the onLoop().

import org.osbot.rs07.script.Script;
import org.osbot.rs07.script.ScriptManifest;

/**
 * Created by Martin on 2017-07-26.
 */
@ScriptManifest(author = "slazter",logo = "",version = 0.1,name = "Banking-Test",info = "BankingClass")

public class Banking extends Script{

    boolean depositPre;

    //no arg constructor supposed to deposit all
    public Banking() throws InterruptedException {
        if(!getBank().isOpen()){
            getBank().open();
        } 
        else if(getBank().isOpen()){
            getBank().depositAll();
        }
    }

    //Suppoted to withdraw the string, and amount
    public Banking(String withdraw,int amount) throws InterruptedException {
        if(!getBank().isOpen()){
            getBank().open();
        }
        else if(getBank().isOpen() && !depositPre && !inventory.contains(withdraw) && getBank().contains(withdraw)){
            getBank().withdraw(withdraw,amount);
        }
        else{
            getBank().close();
        }
    }

    //Supposed to deposit all, then withdraw the string, and amount
    public Banking(String withdraw,int amount,boolean depositPre) throws InterruptedException {
        if(!getBank().isOpen()){
            getBank().open();
        }
        else if(getBank().isOpen() && depositPre && !inventory.contains(withdraw) && getBank().contains(withdraw) ){
            if(getBank().depositAll()){
                if(inventory.isEmpty()){
                    getBank().withdraw(withdraw,amount);
                }
            }
        }
        else{
            getBank().close();
        }
    }

    //Supposed to withdraw the items in stringarray, and amount
    public Banking(String[] withdraws, int amount) throws InterruptedException {
        if(!getBank().isOpen()){
            getBank().open();
        }
        else if(getBank().isOpen() && !depositPre && !inventory.contains(withdraws) && getBank().contains(withdraws)){
                if(!getInventory().contains(withdraws)){
                    for(String d : withdraws){
                        getBank().withdraw(d,amount);
                    }
                }
            }
        else{
            getBank().close();
        }
    }

    //Supposed to deposit all, then withdraw the items in stringarray, and amount
    public Banking(String[] withdraws, int amount, boolean depositPre) throws InterruptedException {
        if(!getBank().isOpen()){
            getBank().open();
        }
        else if(getBank().isOpen() && depositPre && !inventory.contains(withdraws)&& getBank().contains(withdraws)){
            if(getBank().depositAll()){
                if(getInventory().isEmpty()){
                    if(!getInventory().contains(withdraws)){
                        for(String d : withdraws){
                            getBank().withdraw(d,amount);
                        }
                    }
                }
            }
        }
        else{
            getBank().close();
        }
    }



    @Override public int onLoop() throws InterruptedException {

        return random(200,300);

    }
}

and to try to initialize my values in the constructor i thought i could pass the values like this,
 

import org.osbot.rs07.script.Script;
import org.osbot.rs07.script.ScriptManifest;


    @ScriptManifest(author = "Slazter", name = "TestClass", info = "TestClass", version = 0.1, logo = "")

    public final class TestClass extends Script {

        @Override
        public final int onLoop() throws InterruptedException {
            Banking banking = new Banking("Amethyst arrow",5);
            return random(150, 250);
        }
    }

 

[Khaleesi]

The NPE happends bcs you are not passing the Script object from your main class, you can extend this banking class as much as you want with "Script", but thats a wrong use of it.

This code is a very weird way of using OOP ... why are you running code in the constructor???
A COnstructor is to initialize variables and such, not to run code that interacts with the game..

[Apaec]

As Khal mentioned above, it's great that you're trying to learn about some of the cool features java provides, but what you're trying to do is a very strange way of using objects! You've made a bunch of different constructors, but the function of classes should probably be contained in the body of the class, not the constructor. I would suggest saving the constructor for initialising variables, setting values, or anything else you need to do to 'construct' an instance of the class. (or you could use an initialisation block for variables :p).

I would suggest that you try a project where the concept of object orientation fits well with the script, for example an AIO or multi-mode script of some sort. 

[slazter]

3 hours ago, Khaleesi said:

The NPE happends bcs you are not passing the Script object from your main class, you can extend this banking class as much as you want with "Script", but thats a wrong use of it.

This code is a very weird way of using OOP ... why are you running code in the constructor???
A COnstructor is to initialize variables and such, not to run code that interacts with the game..

Hmm okay i understand then, constructors are for initializing values, and not to run code itself, got it . Would it make any sense to have the same banking class, but instead of having it made as constructors, have them made as methods? I mean shouldn't there be some sense in having a banking class as to not having to type the same methods all over everytime i want to do some generic banking in my scripts?
Also,  "you are not passing the Script object from your main class" would you care to explain further?, wouldn't me extending the script, or mabye methodProvider mean that i can inherit the banking methods in another class that also extends Script, and therefore are able to use them? 

 

 

3 hours ago, Apaec said:

As Khal mentioned above, it's great that you're trying to learn about some of the cool features java provides, but what you're trying to do is a very strange way of using objects! You've made a bunch of different constructors, but the function of classes should probably be contained in the body of the class, not the constructor. I would suggest saving the constructor for initialising variables, setting values, or anything else you need to do to 'construct' an instance of the class. (or you could use an initialisation block for variables :p).

I would suggest that you try a project where the concept of object orientation fits well with the script, for example an AIO or multi-mode script of some sort. 

Ohh nice an answer from @Apaec, i've read your guide countless of times!! Really sparked my drive to learn Java!  Okay so function of classes contained inside the body, and initializing variable and setting for constructors, got it! 

" I would suggest that you try a project where the concept of object orientation fits well with the script, for example an AIO or multi-mode script of some sort. ". Yeh spot on, that's the end plan but i figured since i use the technique you provided, getState, and then switch between states, it'd make for even more easy to read and manageable code if i we're to have like a banking class, a walking to Position class, Eating in certain generic conditions and such. Now i often have duplicate methods in many of my scripts, (like banking, which could be made more generic and then @overriden right?) which feels kinda dumb.
Isn't there some sort of way i can "add" methods, to like the methodprovider, that all classes can use, without having to use interfaces, which would kinda defeat my purpose of rewritting the same code all over?

also, when you say Multi-mode or AIO could u give me some example? Not the code but perhaps some of the structure? 
Cause that's exactly what i want to do, but don't think i understand how to extend correctly?
Cause my understanding of this is something like this, 
Say i want to do a progressive Slayer script,
Then mabye i'd want to have acess to banking methods, so i need a banking class(bank loot, restock etc) ,i want walking methods so i need a walking class(to masters,monsters), fighting methods so, Fighting class(fighting monsters) etc,
Then say bank class extends script, walking extends banking, fighting extends walking, and then the Slayer script extends walking, to inherit all those methods?
Otherwise i'd run into the deadly diamond of death. But this feels wrong because they don't pass the IS-A test, fighting isn't a walking... and neither is walking a banking. 

And i know im supposted to be able to do something like this, cause in this post, he ussd task based script, which i felt kinda were almost the same thing as using enums for States, and then switch between states, Please correct me if im wrong. So i know i somehow should be able to have my scripts being able to inherit methods without having to retype it. 

What would be a correct way to go about thinking about this kind of thing?  Sorry if i've made myself unclear somewhere , I'm still reading the Heads on Java book, and reading guides. 

Edited July 26, 2017 by slazter

[gumibearscuz]

Quote

Also,  "you are not passing the Script object from your main class" would you care to explain further?, wouldn't me extending the script, or mabye methodProvider mean that i can inherit the banking methods in another class that also extends Script, and therefore are able to use them? 

The idea behind the "Script" class is that it provides a lot of stuff to run your class as a script. This means that your main class should extends Script so it runs but it also means that you shouldn't just have any class extends Script to get access to the OSBot API. One common way to handle this is to pass a MethodProvider to each new object you make like so:

BankingClass banker = new BankingClass( (MethodProvider)this );

If this code is run in your main class (that extends Script) then it will construct a BankingClass object and give it the MethodProvider (the script class extends MethodProvider so we can cast the Script up to its parent class, MethodProvider). Now in the BankingClass I can refer to its MethodProvider to access any cool functions. Another way to handle this is to make a class with a static MethodProvider and then updating that from your main class (I recommend looking up how static variables/functions work vs member variables and functions, if this doesn't make a whole lot of sense).

 

Quote

Say i want to do a progressive Slayer script,
Then mabye i'd want to have acess to banking methods, so i need a banking class(bank loot, restock etc) ,i want walking methods so i need a walking class(to masters,monsters), fighting methods so, Fighting class(fighting monsters) etc,
Then say bank class extends script, walking extends banking, fighting extends walking, and then the Slayer script extends walking, to inherit all those methods?
Otherwise i'd run into the deadly diamond of death. But this feels wrong because they don't pass the IS-A test, fighting isn't a walking... and neither is walking a banking. 

Well first off, getWalking() and getBank() have support for most common walking/banking features so you don't necessarily need your own classes for them but you could make them if you want custom/extended functionality.

Second, you are getting into design decisions here which honestly is a really hard part of programming. I cannot simply teach you the best way to design and layout your project, you generally learn that skill through trial and error.

The way I would think about it (not saying this is the best/cleanest way, simply what I personally would do) is make a task/node based script where you have different nodes/tasks for each of these things instead of classes that extend things. E.g. you want to make a mining script: task1 = mineOres, task2 = dropOres. And then add whatever code specific to mining an ore or dropping an ore in those tasks/nodes. 

[Alek]

A lot of scripters on here are going to teach you bad practices, and you're going to learn bad programming. 

 

The official way of initializing MethodProvider in your own API is to exchange context, not passing the Script instance. For whatever reason, someone figured out some hacky solution ages ago and everyone learned from that.

The correct way:

Pass "Bot" to a new class. Have this class contain a new instance of MethodProvider or have it extend MethodProvider. Now use "exchangeContext(Bot)". Alternatively you could just pass the bot instance and call your methods directly, aka "bot.getMethods().getCombat().method()".

 

Second, "this" keyword is used for disambiguation. You only use it for something like this:

class test {
boolean x = true;
test(boolean x) {
this.x = x;
}

If you're using it as a personal preference/coding style, then don't. A lot of try hards who are new to programming do "this". 

 

As for task/node/whatever, a lot of scripters completely disregard all optimization when using this style. For instance there will be some "should activate" method, and then in the executor the same code exists. For instance:

 

shouldActivate() {
Objects obj = objects.closest("Tree");
return obj != null;
}

execute() {
Objects obj = objects.closest("Tree");
obj.interact("Chop");
}

 

In this example "Tree" is being searched for twice which is very inefficient. You can avoid these bad practices, but you should really spend time understanding programming first. State logic would be a good stepping stone. 
 

[slazter]

On 2017-07-27 at 7:47 AM, gumibearscuz said:

The idea behind the "Script" class is that it provides a lot of stuff to run your class as a script. This means that your main class should extends Script so it runs but it also means that you shouldn't just have any class extends Script to get access to the OSBot API. One common way to handle this is to pass a MethodProvider to each new object you make like so:

BankingClass banker = new BankingClass( (MethodProvider)this );

If this code is run in your main class (that extends Script) then it will construct a BankingClass object and give it the MethodProvider (the script class extends MethodProvider so we can cast the Script up to its parent class, MethodProvider). Now in the BankingClass I can refer to its MethodProvider to access any cool functions. Another way to handle this is to make a class with a static MethodProvider and then updating that from your main class (I recommend looking up how static variables/functions work vs member variables and functions, if this doesn't make a whole lot of sense).

 

Well first off, getWalking() and getBank() have support for most common walking/banking features so you don't necessarily need your own classes for them but you could make them if you want custom/extended functionality.

Second, you are getting into design decisions here which honestly is a really hard part of programming. I cannot simply teach you the best way to design and layout your project, you generally learn that skill through trial and error.

The way I would think about it (not saying this is the best/cleanest way, simply what I personally would do) is make a task/node based script where you have different nodes/tasks for each of these things instead of classes that extend things. E.g. you want to make a mining script: task1 = mineOres, task2 = dropOres. And then add whatever code specific to mining an ore or dropping an ore in those tasks/nodes. 

Hmm so if my understanding is correct, what we're trying to do with that snippet is:
1)Making a Object from the Banking class in my main main script that extends Script.
2)Have the banking methods be sent to the MethodProvider, via a constructor and because Scripts extend MethodProvider(therefore pseudo adding them i guess you could call it?) we should now have acess to those methods in the main script that extends Script?
Is this understanding somewhat correct? 

I tried the snippet, but didn't work, didn't work if i put this. pre or after, aswell as both removing, or adding more "()". aswell as having the banking class itself extend either Script or Methodprovider.
Think im gonna have to look into what you've said about static variables/functions work vs member variables and functions. I feel my understanding of that is really lackluster, haven't gotten to that part in my Java book yet. 

Also yeh, i'll probably look into Task based programming later on, i'm kinda intrigued by it,  as i saw it requires abstract classes, which i think would be a good practice to learn more about making abstract classes, which hopefully leads help me to understand on how to write interfaces, then functional interfaces, and then some sick lambdas  

 

 

On 2017-07-27 at 8:32 AM, Alek said:

A lot of scripters on here are going to teach you bad practices, and you're going to learn bad programming. 

 

The official way of initializing MethodProvider in your own API is to exchange context, not passing the Script instance. For whatever reason, someone figured out some hacky solution ages ago and everyone learned from that.

The correct way:

Pass "Bot" to a new class. Have this class contain a new instance of MethodProvider or have it extend MethodProvider. Now use "exchangeContext(Bot)". Alternatively you could just pass the bot instance and call your methods directly, aka "bot.getMethods().getCombat().method()".

 

Second, "this" keyword is used for disambiguation. You only use it for something like this:

class test {
boolean x = true;
test(boolean x) {
this.x = x;
}

If you're using it as a personal preference/coding style, then don't. A lot of try hards who are new to programming do "this". 

 

As for task/node/whatever, a lot of scripters completely disregard all optimization when using this style. For instance there will be some "should activate" method, and then in the executor the same code exists. For instance:

 

shouldActivate() {
Objects obj = objects.closest("Tree");
return obj != null;
}

execute() {
Objects obj = objects.closest("Tree");
obj.interact("Chop");
}

 

In this example "Tree" is being searched for twice which is very inefficient. You can avoid these bad practices, but you should really spend time understanding programming first. State logic would be a good stepping stone. 
 

Hmm so exchanging content and not passing the script instance.. is what i'd be doing in the previous part, where i send my object to Methodprovider a passing of the script instance?

Hmm okay so let me se if i understand correctly, i'm supposted to pass bot to a new class, and have that class extend MethodProvider. 
Would that be something like this?

   Banking banking = new Banking(bot);

and in my Banking class, have it extend Methodprovider. Cause i tried this, and it won't work, so i think im passing wrong, so i googled on how to pass variables to classes.
https://stackoverflow.com/questions/18194661/java-pass-variable-from-class-to-class
and i could use Getters and Setters, 
so i made methods for getters and setters in bot classes, still didn't work, think im gonna have to read up on, passing objects to other classes, and not just instance variables..

Regarding "this", yeh okay noted  

And regarding Node and Task based, im super intrigued by the making of them, it seems to make alot of sense, and then i guess i could have my banking methods as a Node, and make the onLoop, super fkn clean  But yeh i'll keep that in mind from now on, even in the scripts i write atm, so as not to have the same condition checked twice, i guess that logic applies for things like: if(!myPlayer.isMoving()) aswell even if i don't search for the object, i guess it still takes some processing to go through it if it's a part of both my getState, and the method for what's supposed to happen in that state.

And yeh your probably right, i did try to get some fundamental understanding of a state machine from the States vs tasks discussion, Where Tom said, 

"What if I told you they are the same thing, just one is more of an object oriented approach and reflects better practice.

End result = State Machine"

I guess the first thing i'll start on doing right now is understanding State logic better, Thanks for the Help!  

[Explv]

10 minutes ago, slazter said:

Hmm so exchanging content and not passing the script instance.. is what i'd be doing in the previous part, where i send my object to Methodprovider a passing of the script instance?

Hmm okay so let me se if i understand correctly, i'm supposted to pass bot to a new class, and have that class extend MethodProvider. 

 

Make your class extend MethodProvider:

public class Banking extends MethodProvider {
   // whatever 
}

Then initialize it (once) like so (has to be done in a class that extends MethodProvider, e.g. your main Script class)

Banking banking = new Banking();
banking.exchangeContext(getBot());

You will now be able to access all the API methods from inside your Banking class.

10 minutes ago, slazter said:

And regarding Node and Task based, im super intrigued by the making of them, it seems to make alot of sense, and then i guess i could have my banking methods as a Node, and make the onLoop, super fkn clean  But yeh i'll keep that in mind from now on, even in the scripts i write atm, so as not to have the same condition checked twice, i guess that logic applies for things like: if(!myPlayer.isMoving()) aswell even if i don't search for the object, i guess it still takes some processing to go through it if it's a part of both my getState, and the method for what's supposed to happen in that state.

"Node", "Tasks", "States" all suck

 

Also regarding your initial post, go follow some Java tutorials, especially an OOP tutorial so you can understand why what you wrote is wrong.

Edited July 28, 2017 by Explv

[Tom]

7 minutes ago, Explv said:

 

"Node", "Tasks", "States" all suck

 

Chances are your scripts are using some form of state logic, I wouldn't say the pattern sucks, I would just say new scripters don't understand why one way is better than the other, even though they all technically accomplish the same thing

[Explv]

47 minutes ago, Tom said:

Chances are your scripts are using some form of state logic, I wouldn't say the pattern sucks, I would just say new scripters don't understand why one way is better than the other, even though they all technically accomplish the same thing

Depends what you mean by state logic, these buzzwords are thrown around so much on here it's hard to tell what people mean by them.

And I do think the patterns themselves suck, I won't rant about it again on this thread, but you can see what I said before here:

 

[slazter]

7 hours ago, Explv said:

 

Make your class extend MethodProvider:

public class Banking extends MethodProvider {
   // whatever 
}

Then initialize it (once) like so (has to be done in a class that extends MethodProvider, e.g. your main Script class)

Banking banking = new Banking();
banking.exchangeContext(getBot());

You will now be able to access all the API methods from inside your Banking class.

"Node", "Tasks", "States" all suck

 

Also regarding your initial post, go follow some Java tutorials, especially an OOP tutorial so you can understand why what you wrote is wrong.

@Explv, Bro your a lifesaver, once again you come to the rescue to save my ass with your vodoo  
So trying to connect this code that you wrote, with what Alek said, to see if i understand correctly,
We're extending the Methodprovider, as to go up in the inheritance tree, so that the Script can inherit it, otherwise, if we we were to extend Script, it'd be on the same "level" in the inheritance tree as the current Script, therefore not being able to acess it. Therefore we have to go up to MethodProvider, but we could go even higher aswell, is that a fair assumption?


WOW i just think i figured it out.. xD Thanks alot, in swedish we say "Poletten trillade ner" xD 
I think you just made me understand that i kinda missunderstod Aleks post.

To itterate my current understanding of what Alek said and from what you've posted.
The correct way to make our methods available to other classes is to pass the bot.
We accomplish that by first extend our class to Methodprovider, Then we use exchangeContext on the referece variable, to pass the actual bot as a argument, with exchangeContext(getbot)

My first understanding was that we,
Pass the bot - didn't get a clue about how to send an entire class to another class x) 
Extend Methodprovider - easy
Then use exchangeContext(getbot) - didn't understand on what.

So it's not 3 steps, it's actually 2 steps.
Would this be a correct understanding?

Also, yeh I'll put that on my list of things to read up on!
That currently contain: 

• Anonyma Inner Classes
• Streams 
• Collections
• Lists
• GUI
• Lambdas
• State logic
• Object Orientering, med inheritance
• Arrays - Speciellt multipla, med Loopar
• Threads

xDD

Any specific tutorial, book or video you had in mind that you feelt we're worth recommending or just read up on it in general?  

 

 

[The Undefeated]

On 7/27/2017 at 8:32 AM, Alek said:

A lot of scripters on here are going to teach you bad practices, and you're going to learn bad programming. 

 

The official way of initializing MethodProvider in your own API is to exchange context, not passing the Script instance. For whatever reason, someone figured out some hacky solution ages ago and everyone learned from that.

The correct way:

Pass "Bot" to a new class. Have this class contain a new instance of MethodProvider or have it extend MethodProvider. Now use "exchangeContext(Bot)". Alternatively you could just pass the bot instance and call your methods directly, aka "bot.getMethods().getCombat().method()".

 

Second, "this" keyword is used for disambiguation. You only use it for something like this:

class test {
boolean x = true;
test(boolean x) {
this.x = x;
}

If you're using it as a personal preference/coding style, then don't. A lot of try hards who are new to programming do "this". 

 

As for task/node/whatever, a lot of scripters completely disregard all optimization when using this style. For instance there will be some "should activate" method, and then in the executor the same code exists. For instance:

 

shouldActivate() {
Objects obj = objects.closest("Tree");
return obj != null;
}

execute() {
Objects obj = objects.closest("Tree");
obj.interact("Chop");
}

 

In this example "Tree" is being searched for twice which is very inefficient. You can avoid these bad practices, but you should really spend time understanding programming first. State logic would be a good stepping stone. 
 

Why is .exchangeContext() deprecated then?

[Explv]

49 minutes ago, The Undefeated said:

Why is .exchangeContext() deprecated then?

It probably shouldn't be. It's marked as internal use only, but l use it in my scripts anyway.

Edited August 7, 2017 by Explv

[The Undefeated]

43 minutes ago, Explv said:

It probably shouldn't be. It's marked as internal use only, but l use it in my scripts anyway.

Yeah, but it's annoying to read like that.