oa2sa - remember to use standard libs and read some manuals

[Max mtg]

I found an awful method forge.control.home.ControlConstructed.oa2sa(Object[]) which is supposed to return a string array of the same size.

Code: Select all

    /**
     * Exhaustively converts object array to string array.
     * Probably a much easier way to do this.
     */
    private String[] oa2sa(final Object[] o0) {
        final String[] output = new String[o0.length];

        for (int i = 0; i < o0.length; i++) {
            output[i] = o0[i].toString();
        }

        return output;
    }

I always get shocked to see some obvious things be done in a very complicated way.
Author = Doublestrike, exists since 12716 (that is for 2 months)

used in scenarios like this:

Code: Select all

    private String[] getEventNames() {
        final List<String> eventNames = new ArrayList<String>();
        eventNames.clear();

        for (final QuestEvent e : Singletons.getModel().getQuestEventManager().getAllChallenges()) {
            eventNames.add(e.getEventDeck().getName());
        }

        for (final QuestEvent e : Singletons.getModel().getQuestEventManager().getAllDuels()) {
            eventNames.add(e.getEventDeck().getName());
        }

        return oa2sa(eventNames.toArray());
    }


Firstly, a newly created list does not need to be cleared (it's already empty)
Second, there is a simple way to convert List<T> to T[]: declare an array of matching size and pass it as argument to toArray() method. Another option: pass there an empty array to have the function allocate array for you.
Third, once you create utility funcitons that might be useful to other classes anr their devlopers, care to find a good place for that code in forge.util package.

So the a better edition for this method would be

Code: Select all

    private String[] getEventNames() {
        final List<String> eventNames = new ArrayList<String>();
        final QuestEventManager qm = Singletons.getModel().getQuestEventManager();
       
        for (final QuestEvent e : qm.getAllChallenges()) {
            eventNames.add(e.getEventDeck().getName());
        }

        for (final QuestEvent e : qm.getAllDuels()) {
            eventNames.add(e.getEventDeck().getName());
        }

        return eventNames.toArray(ArrayUtils.EMPTY_STRING_ARRAY);
    }

This is what checkstyle should really check.

I also think that this forum can be a good place to ask questions on how to do something. Java questions like "how to convert list to array" will find answers here for sure, even if same question had been asked on stackoverflow decades ago.
As for me it's better to answer a simple question here than to see utility functions custom implementations in our project.

[jeffwadsworth]

Thanks for the free lesson. Speaking for myself (non-programmer), I add simple code which is 95% based off of previous work...just adding or fixing some functionality. If it is inefficient, I assume that the guru's will fix it. Card scripts work that way. We all have our areas of expertise.

[Sloth]

Only to set things into relation, i advice you to take a look at the function getSpellCostChange Max. And we had much worse code.

[Doublestrike]

edit - on second thought, not going to bite . Thanks for the tip, it's fixed.

[moomarc]

Like Jeff, I'm a non-coder, but I can generally see basic patterns in the code that enables me to add some small functions. If two bits of code seem to handle similar functions in different ways, I'll use the one that looks cleaner as a reference (although it might not be the most efficient code). I always post on the forum though to alert the professionals to what I've done so that it can be checked and cleaned up if necessary (for instance when I added support for Glissa Sunseeker, friarsol pointed out a much cleaner method). I just feel that the professionals can't get around to everything so every small bit that I can manage myself helps.

I'm slowly learning though, and appreciate tips like these.

[jeffwadsworth]

I would like to do the following without reinventing the wheel.

Script for Ward Sliver:

| Open

Name:Ward Sliver
ManaCost:4 W
Types:Creature Sliver
Text:no text
PT:2/2
T:Mode$ ChangesZone | ValidCard$ Card.Self | Origin$ Any | Destination$ Battlefield | Execute$ ChooseColor | Static$ True | TriggerDescription$ As CARDNAME enters the battlefield, choose a color.
SVar:ChooseColor:DB$ ChooseColor | Defined$ You
S:Mode$ Continuous | Affected$ Creature.Sliver | AddKeyword$ Protection:Card.ChosenColor:CARDNAME has protection from the chosen color. | Description$ All Slivers have protection from the chosen color.
SVar:RemAIDeck:True
SVar:Rarity:Uncommon
SVar:Picture:http://www.wizards.com/global/images/magic/general/ward_sliver.jpg
End

Now, I think the best way to handle this would be to simply replace "ChosenColor" with hostCard.getChosenColor() within the addKeyword section of StaticAbilityContinuous.java. "addKeyword" is a String[] so I tried this:

Code: Select all

Arrays.toString(addKeywords).replace("ChosenColor", CardUtil.getShortColorsString(hostCard.getChosenColor()));

Which obviously does not work. Do you guys know of an elegant method for accomplishing this goal?

[Max mtg]

iterate over the array, replace ChosenColor in each string