Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Demonstrative Example #572

Closed
wants to merge 3 commits into from
Closed

Demonstrative Example #572

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
99527dd
Update README.markdown
Vinit-Pandit Apr 23, 2024
ff6397d
Update README.markdown
Vinit-Pandit Apr 24, 2024
8f45660
Merge branch 'main' into main
Vinit-Pandit Apr 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions README.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,42 @@ standards such as [JSON Schema](http://json-schema.org), [JSON
Pointer](https://www.rfc-editor.org/rfc/rfc6901),
[JSONL](https://jsonlines.org), and more.

Example
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need this, mainly if its not really a title. Maybe just end the above paragraph with "For example:"?


```cpp
#include <sourcemeta/jsontoolkit/json.h>
#include <sourcemeta/jsontoolkit/jsonpointer.h>
#include <sstream>
#include <iostream>

int main() {
// Creating JSON using parse
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Creating JSON using parse
// Parsing a JSON document

sourcemeta::jsontoolkit::JSON document =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the README we likely have more horizontal space, so you can format this a bit nicer:

sourcemeta::jsontoolkit::JSON document = sourcemeta::jsontoolkit::parse(R"JSON({
  "name": "John Doe",
  "age": 20,
  "address": "zxy"
})JSON");

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tony-go It'd have been so cool to have #506 here 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I do bad dreams because of this hahaha :)

sourcemeta::jsontoolkit::parse(R"JSON({
"name": "John Doe",
"age": 20,
"address": "zxy"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe put a short semi-real address? I guess you can generate a supposedly valid i.e. US address on Google

})JSON");

// JSON pointer for the name property
const sourcemeta::jsontoolkit::Pointer name_pointer{"name"};
// A new JSON document for the name
const sourcemeta::jsontoolkit::JSON name_value{"Johnny Doe"};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const sourcemeta::jsontoolkit::JSON name_value{"Johnny Doe"};
const sourcemeta::jsontoolkit::JSON new_value{"Jane Doe"};

I think "Jane Doe" is a more classic one

// Updating the value of the name using JSON pointer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Updating the value of the name using JSON pointer
// Updating the value of the name property using a JSON pointer

sourcemeta::jsontoolkit::set(document, name_pointer, name_value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sourcemeta::jsontoolkit::set(document, name_pointer, name_value);
sourcemeta::jsontoolkit::set(document, {"name"}, new_value);

I think inlining the JSON Pointer looks cooler from an example point of view

sourcemeta::jsontoolkit::prettify(document, std::cout);
std::cout<<"\n";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::cout<<"\n";
std::cout << "\n";

Be mindful of style!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅 It's going to take me some time to develop such detailed styled practices.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We all went through it :) Overall, attention to detail is super, super important in software engineering


// The above program will print the following to standard output:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I see this again, I wonder if you should do this AFTER the code block. i.e. after showing the C++ program, write on the README: "The above program will print the following to standard output:", then open a new JSON code block, and put the output?

//output :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no space between // and output here. In any case, I don't think you need to say "output :" here if the above sentence always says that the program will print the following to standard output

// {
// "address": "zxy_with_bar",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, there is a mismatch between this and the actual example above

// "age": 21,
// "name": "xyz_with_foo"
// }
}
```

Documentation
-------------

Expand All @@ -22,3 +58,5 @@ long the outcome is distributed under the same license. Otherwise, you must
obtain a [commercial license](./LICENSE-COMMERCIAL) that removes such
restrictions. Read more about our licensing approach
[here](https://www.sourcemeta.com/licensing/).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This blank lines should go away


Loading