Object:perform

Development
1

Continuing the discussion from How do I trace error? @jamshark70 @jordan

Object:perform usually process an Array whose first element is a Symbol (‘selector’), with exceptions without a symbol.

First, expects its first element to be a Symbol acting as the ‘selector’. However, the method also handles cases where the selector is not a symbol. Such flexibility, while powerful to play with other things, introduces complexity and can indeed be a source of bugs and terrible error messages.

–The last bug we talked about here was with RawArray subclasses, on the other thread. Where it just checks the selector, and does not do any kind of type checking on the actual values.–

my comments (any mistake is my bad)

// Function to "do stuff on objs"
int objectPerform(struct VMGlobals *g, int numArgsPushed)
{
    // Making some vars to hold 
    PyrSlot *recvrSlot, *selSlot, *listSlot;
    PyrSlot *pslot, *qslot;
    PyrSymbol *selector;
    int m, mmax;

    // Figuring out where the message's meant to go based on args
    recvrSlot = g->sp - numArgsPushed + 1;
    // Selector's the next dude?
    selSlot = recvrSlot + 1;
    if (IsSym(selSlot)) { // If selector's a symbol, we're cool
        selector = slotRawSymbol(selSlot); // Grab the actual symbol from the slot
        // Gotta shuffle args down a slot to fill where selector was beforee
        pslot = selSlot - 1;
        qslot = selSlot;
        for (m=0; m<numArgsPushed - 2; ++m) slotCopy(++pslot, ++qslot);
        g->sp --; // Fix the stack pointer
        numArgsPushed--; // One less arg to worry bout
        // Now it looks just like a normal msg 
    } else if (IsObj(selSlot)) { // If selector's an object...
        listSlot = selSlot;
        // If it's a list, we need to get its slots
        if (slotRawObject(listSlot)->classptr == class_list) {
            listSlot = slotRawObject(listSlot)->slots;
        }
        // If it ain't an object or ain't an array, it's no good
        if (NotObj(listSlot) || slotRawObject(listSlot)->classptr != class_array) {
            goto badselector;
        }
        // Get the array from the list slot
        PyrObject *array = slotRawObject(listSlot);
        if (array->size < 1) { // Needs at least one selector inside
            error("Array must have a selector.\n");
            return errFailed;
        }
        selSlot = array->slots; // Grab the selector from array slots
        selector = slotRawSymbol(selSlot); // Get the real symbol for selector

        // Shuffle args if more than 2
        if (numArgsPushed>2) {
            qslot = recvrSlot + numArgsPushed;
            pslot = recvrSlot + numArgsPushed + array->size - 2;
            for (m=0; m<numArgsPushed - 2; ++m) slotCopy(--pslot, --qslot);
        }

        // Copy args from array to where they go
        pslot = recvrSlot;
        qslot = selSlot;
        for (m=0,mmax=array->size-1; m<mmax; ++m) slotCopy(++pslot, ++qslot);

        // Fix the stack and how many args we're dealing
        g->sp += array->size - 2;
        numArgsPushed += array->size - 2;
        // Setup's good, it seems... ?
       // but it can also be very wrong, with wrong types in RawArray, for example

    } else { // If selector's messed up, it's BAD
        error("perform selector not a Symbol or Array.\n"); // Show us the problem. 
        dumpObjectSlot(selSlot);    // better error messages, babe..((

        return errWrongType;
    }

    // Finally, send the msg with the selecto, and arg count
    sendMessage(g, selector, numArgsPushed);
    g->numpop = 0; // Reset the pop count
    return errNone; 

// that's an intricate path... ))) the upside is that it's flexible

That’s an intricate path… ))) the upside is that it’s flexible. That’s what we want? But can’t it be improved? RawArray is a case to improve, for example. Many alternatives are possible, of course.

2

Some general ideas:

Devs should try to comment and make the C++ code more readable. Sometimes, it seems it tries to obscure the purpose of the code; good comments, with explanations, are also good documentation and will avoid many kinds of problems (even among devs themselves) – (See the source code of GHC; most of it are COMMENTS. Let’s learn from it)

Using more structured approaches instead of’ goto’ is cleaner than jumping to error handling. Consider using if-else statements or switching to a system that checks and handles errors as they pop up. It’s like having a safety net rather than a panic button.

Especially for things like RawArray, where it’s in SC, you need to type-check, and SC just doesn’t care. Make sure your type checks are on point. If nothing is checked, all sorts of crazy things happen.

What do we need to keep in mind when writing tests for this?

Just some ideas…

3 
var message = ['+', 3];

1.perform(message);
-> 4

Gosh. 20+ years and I had no idea that could work :laughing:

It’s a lot less nonsensical when explained with a descriptive variable name :wink:

hjh

4

@jamshark70 Was that ironic (you know that for 20 years), or are you saying it’s undocumented and could improve? I didn’t get your meaning.

The function is not straightforward for me at all )))

5

I genuinely did not know about this, and I’d have no expectation that any other user would have guessed.

hjh

6

The fact that these methods let you pass an array which is expanded internally, I don’t think is necessary and just makes things confusing, particularly around variable args. Using ‘*’ to expand the arguments manually is very simple and explicit.

When working on doesNotUnderstandWithKeys

I’ve found none of these methods supported keyword args.

Instead we settled on making a new method perform/valueWith(selector, argumentsArray, keywordArgumentPairs)

Which doesn’t expand the arguments and I think is much clearer and explicit than any of the alternatives.

When I get a chance, I’m going to play around with replacing all calls to performArray/List/Envir/WithEnir… with a variation of performWith. They all basically try to do the same thing and unifying them would cut down on the amount code that needs to be maintained.

1 Like
7

I made an issue thinking this was a bug…

8

I tried to write a function that would pass the array; you can review it probably there is something incorrect)))

I think there must be a much better design. One function like this can’t be able to do all the things required for an optimal situation, such as type-checking, error handling, safety, good error messages, etc.

9

Ah, interesting. I’ll take a look at your code another time. It’s something different.