You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "pdxcodemonkey (GitHub)" <gi...@apache.org> on 2018/11/01 17:10:20 UTC
[GitHub] [geode-native] pdxcodemonkey opened pull request #389:
GEODE-4337: example cpp function execution
- Simplified example to only call one server function
- Added example.jar file containing only necessary function
- Stopped installing test javaobject.jar file
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
What isn't wrong with this statement.
First, use `=` with `auto`:
```
auto value = CacheableString::create(valueStream.str().c_str());
```
Second, don't convert to `const char *`:
```
auto value = CacheableString::create(valueStream.str());
```
Third, don't explicitly create the `CacheableString`:
```
```
See comment below.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Still need this?
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
`const`?
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
You can put primitive types here.
```
auto key = std::string("KEY--") + std::to_string(i);
auto value = std::string("VALUE--") + std::to_string(i);
region->put(key, value);
```
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on pull request #389:
GEODE-4337: example cpp function execution
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
Fixed.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
What is a `regPtr0`? Use good names, like `region`.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Why suffix variables with `Ptr`, just call it `region`.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on pull request #389:
GEODE-4337: example cpp function execution
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
Fixed
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on pull request #389:
GEODE-4337: example cpp function execution
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
Nope.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Why not have the key and values be more relatable than this. Like how does "KEY--1" relate to "VALUE--3". Seems so random.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on pull request #389:
GEODE-4337: example cpp function execution
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
Um, because I just lifted existing Java code from javaobject.jar?. I'll clean this up.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey closed pull request #389:
GEODE-4337: example cpp function execution
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
[ pull request closed by pdxcodemonkey ]
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Does this still have a place now that we have fixed set of keys and values?
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on pull request #389:
GEODE-4337: example cpp function execution
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
At first blush it doesn't appear to work. GEODE-6004 filed for tracking.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on pull request #389:
GEODE-4337: example cpp function execution
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
Fixed
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on pull request #389:
GEODE-4337: example cpp function execution
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
Yep
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Why every other value? Why not fixed values here. This stinks like the integration test. This should be a good clean room function example.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on pull request #389:
GEODE-4337: example cpp function execution
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
Yeah, just changed values to 1,2,3 as well - makes more sense, at least they agree now.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey closed pull request #389:
GEODE-4337: example cpp function execution
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
[ pull request closed by pdxcodemonkey ]
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Stop the `Ptr` suffix.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Why subscriptions in a function example?
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on pull request #389:
GEODE-4337: example cpp function execution
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
Removed
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on pull request #389:
GEODE-4337: example cpp function execution
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
Using fixed keys and values cleans this up nicely - thanks
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
This "KEY--" stinks of the bad integration test you took this example from.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
What's a `queryObject`? The method is `withArgs` so why not use a variable name that matches? `arguments`?
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Why not use fixed keys and values. What's the point of the looping and the concatenation.
If you insist on using derived values here then use `std::to_string`:
```
auto key = std::string("KEY--") + std::to_string(i);
```
`std::stringstream` is overkill.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Import what you need, not *.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on pull request #389:
GEODE-4337: example cpp function execution
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
Nope - removed.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#389: GEODE-4337: example cpp function execution
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Why do we have different types in this argument checking? Your example only sends strings.
[ Full content available at: https://github.com/apache/geode-native/pull/389 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org