search

sstrigler / JSJaC

JavaScript Jabber Client Library
Other
295 stars 86 forks source link

JsJacMessage GetNode #50

Closed eloycoto closed 11 years ago

eloycoto commented 11 years ago

Hi,

In JsjacMessage the node values is not possible to get from the client, this is a example.

Generate new node

    var customValues = {
        one:1,
        two:2
    }

    var oMsg = new JSJaCMessage();
    oMsg.buildNode("newNode");
    oMsg.appendNode("newNode",customValues);
    oMsg.setTo(new JSJaCJID(sendmessage));
    oMsg.setBody(sendtext);
    con.send(oMsg);

This is the xml:

<message xmlns='jabber:client' to='test@192.168.100.68' from='eloy@192.168.100.68/17579aa1-0e71-4ca6-87e2-b6252d1cb3fb'><newNode one='1' two='2'/><body>hello</body></message></body>

In the other side of the customer, you only need use this to get all attributes:

function handleMessage(oJSJaCPacket) {
    var attrs = oJSJaCPacket.getNodeVal("newNode");
    ... 
}

Cheers

rraptorr commented 11 years ago

Leaving calls to console.log is not very nice (apart from polluting console it will crash in IE8 and lower as there is no console.log there).

eloycoto commented 11 years ago

Yes sure, excuse me. I'm defining always console.log in my js apps.

Cheers.

rraptorr commented 11 years ago

Then don't you think that you should update your pull request?;)

sstrigler commented 11 years ago

Some remarks ahead.

From my understanding what your code does, shouldn't the function rather be called getNodeAttrs() or the like?

Also I have to admit I don't quite get why you're using this strange looking construct of 'Array.prototype.slice.call'. Is this to make sure the array-like attributes property is converted to Array properly? If so there should be a comment explaining this.

Also you could/should have used getChild(...) to retrieve the node for consistence.

Last but not least personally I see no good reason to have a helper functions like this included with JSJaC. What you're doing here is converting from W3C Dom to JSON. Which of course might be a convenient thing to do. Currently we only have this for the other direction using buildNode and appendNode. My problem now is that if we have one function of this kind just for attributes then shouldn't we rather offer a generic interface to convert from W3C Dom to JSON? I'd say that"s nice to have but out of scope for JSJaC and can be done easily by some generic third party library. Also you can use jQuery to make it easier to deal with W3C DOM (e.g. $(oJSJaCPacket.getNode()).attr(...)).

eloycoto commented 11 years ago

Hi,

Yes the function name should be getNodeAttrs() more readable name, I use the same format at getChildVal()

Array.prototype.slice.call always work for me, I use this function in the lasts years and never have a problem with this.

Sure you can use getChild to get all information. I use json objects, that in the appendNode you use a Json object, I want the same result :) I use json objects for emberjs integration.

I send the pull request to add a function to a library, in this case for me is useful. I prefer have the function (in thi case) in the jsjac lib that use external libs.

Many thanks for your time. Cheers

rraptorr commented 11 years ago

I'd also like to point out that there is no forEach function in IE8 and lower.