Improvements?

Any way to improve this, or is it fine how it is?

Code:

---

        case 2560:
                if (playerLevel[17] >= 20)
                        {
                        setAnimation(832);
                        addItem(silk_Stall, 1);
                        EXP(0, 17);
                        MSG("You steal from the Silk stall.");
                }
        else if (playerLevel[17] < 20)
                {
                text("Your thieving level isn't high enough to steal from this stall.");
        }
        break;

---

EXP = addSkillXP
MSG = sendMessage
text = Show dialogue with text.

Quote:

---

Originally Posted by Kastro [Only registered and activated users can see links. Click Here To Register...]

Any way to improve this, or is it fine how it is?

Code:

---

        case 2560:
                if (playerLevel[17] >= 20)
                        {
                        setAnimation(832);
                        addItem(silk_Stall, 1);
                        EXP(0, 17);
                        MSG("You steal from the Silk stall.");
                }
        else if (playerLevel[17] < 20)
                {
                text("Your thieving level isn't high enough to steal from this stall.");
        }
        break;

---

EXP = addSkillXP
MSG = sendMessage
text = Show dialogue with text.

---

Well; Personally there are a few things I would change.

• Change 'silk_Stall' to 'SILK_STALL' - Seeing as the item has a definite value, it should be given a name in all caps.
• Remove the 'MSG', 'EXP' and 'text' methods. Creating new methods just to shorten the length of a method name is pointless, not to mention method names shouldn't be in cap.
• Get rid of the 'else if' condition. Just make it 'else'. If a whole number is less than 20, it' going to automatically be good. All that you're doing with checking if it's less than 20 is adding more work on your PC.

Personally; I would have written it like this, or similar to this.

Code:

---

case 2560: //Quick Description here. 'Silk Stall Click'
        if(playerLevel[17] >= 20) {
                setAnimation(832);
                addItem(SILK_STALL, 1);
                addSkillExp(0, 17);
                sendMessage("You still from the silk stall.");
        } else sendMessage("Your thieving level isn't high enough to steal from this stall.");
        break;

---

Quote:

---

Originally Posted by PSNB [Only registered and activated users can see links. Click Here To Register...]

Well; Personally there are a few things I would change.

• Change 'silk_Stall' to 'SILK_STALL' - Seeing as the item has a definite value, it should be given a name in all caps.
• Remove the 'MSG', 'EXP' and 'text' methods. Creating new methods just to shorten the length of a method name is pointless, not to mention method names shouldn't be in cap.
• Get rid of the 'else if' condition. Just make it 'else'. If a whole number is less than 20, it' going to automatically be good. All that you're doing with checking if it's less than 20 is adding more work on your PC.

Personally; I would have written it like this, or similar to this.

Code:

---

case 2560: //Quick Description here. 'Silk Stall Click'
        if(playerLevel[17] >= 20) {
                setAnimation(832);
                addItem(SILK_STALL, 1);
                addSkillExp(0, 17);
                sendMessage("You still from the silk stall.");
        } else sendMessage("Your thieving level isn't high enough to steal from this stall.");
        break;

---

---

I suppose not making it check would be better. Your way is rather messy though and doesn't use the dialogue.

I'll get to work on it.

EDIT: Thought I'd add, Full caps on the array is sort of pointless, I like to declare things like statements.

Code:

---

        case 2560:
                if (playerLevel[17] >= 20)
                        {
                        setAnimation(832);
                        addItem(silk_Stall(), 1);
                        EXP(0, 17);
                        MSG("You steal from the Silk stall.");
                }
        else {
                text("Your thieving level isn't high enough to steal from this stall.");
        }
        break;

---

A little better I suppose?

Quote:

---

Originally Posted by Kastro [Only registered and activated users can see links. Click Here To Register...]

I suppose not making it check would be better. Your way is rather messy though and doesn't use the dialogue.

I'll get to work on it.

EDIT: Thought I'd add, Full caps on the array is sort of pointless, I like to declare things like statements.

---

Keep in mind I haven't the slightest clue what your methods and variables are.

My way is not sloppy, It just looks that way because I'm using variables you were using. If it were on my source, it'd look something like this. Keep in mind, this is all written on the fly.

Code:

---

case SILK_STALL:
                            if(p.getSkills().getThievingLevel() < 20) {
                                    Frames.addChatboxText(p, "You need a thieving level of 20 to steal from this stall.");
                                    break;
                            }
                           
                            if(p.getLocation().isGuardNearby()) {
                                    NPC guard = NPCManager.getNearestNPC(GUARD_ID, p);
                                    guard.triggerSpeech("Oi! What do you think you're doing!?");
                                    guard.attackPlayer(p);
                                    break;
                            }
                            p.getSkills().addExperience(Skills.THIEVING, 45);
                            p.getModel().executeAnimation(832);
                            Frames.addChatboxText(p, "You steal a piece of silk.")
                            break;

---