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.

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

New Tutorial #190

Merged
merged 22 commits into from
Apr 23, 2014
Merged

New Tutorial #190

merged 22 commits into from
Apr 23, 2014

Conversation

jpsim
Copy link
Contributor

@jpsim jpsim commented Apr 16, 2014

I've created a new sample project that showcases how to create a UITableViewController backed by TightDB.

I've marked this as [WIP] because I have yet to add the comments to generate documentation in the source and I'd like to solicit feedback on the sample code itself.


- (NSInteger)tableView:(UITableView *)tableView numberOfRowsInSection:(NSInteger)section {
__block NSInteger rowCount = 0;
[[TDBContext contextWithDefaultPersistence] readTable:kTableName usingBlock:^(TDBTable *table) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't want to create a new context every time. It is not wrong, but there will be some overhead there. It will be much better to add the context as a property and just keep using the same one.

Also, this is a clear case where you would want (need) to use Implicit Transactions. Otherwise you will cannot be sure that the rowCount you obtained will also be valid in the methods that gets the individual rows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll update this to use Implicit Transactions.

@jpsim
Copy link
Contributor Author

jpsim commented Apr 16, 2014

I've updated the project to use implicit transactions. The problem though is that I need a pretty ugly dispatch_after hack to give the TDBSmartContext enough time to propagate its changes. I'm hoping that @astigsen can point me in the right direction here.

@alazier
Copy link
Contributor

alazier commented Apr 17, 2014

This is a design flaw with the initial implementation of implicit transactions. There is no way to manually end the current transaction and to start a new one which can see the latest changes. The simplest way to address this would be to add a method to SmartContext called endCurrentTransaction which would do just this and allow you to see your changes immediately. Alternatively, we could update the read transaction to the latest tree automatically in the case there is a committed write transaction within a read transaction (which is the case here). This would make your example work without the dispatch but still breaks from current model as designed and is kind of hard to understand. But if two hard to understand things cancel each other out maybe it doesn't matter that they happened.

Another alternative would be to change the model we use completely - something I suggested in the past was to wrap individual calls with transactions - if a call is made without a transaction, it is automatically wrapped with one. This model would solve this issue, and would also work with background threads. With this model you do run into an issue where other threads may be altering the same data simultaneously, and in cases where this is a possibility you would have to manually create a transaction to get a consistent view of your data, but I still find this preferable to having to dispatch_after, and I like the fact that it is a single consistent model that works on all threads.

@jpsim
Copy link
Contributor Author

jpsim commented Apr 17, 2014

Thanks for those clarifications, @alazier. I had a nice chat with @astigsen this morning and it seems that my latest commit is probably the best way to do things using the current objc interface.

Also that once sync-style merge rules are in place, writes will also be implicit and we'll no longer need to update via notifications or to wrap the insert/delete in dispatch calls.

@alazier alazier closed this Apr 17, 2014
@jpsim
Copy link
Contributor Author

jpsim commented Apr 17, 2014

Any particular reason why this PR is now closed?

@alazier
Copy link
Contributor

alazier commented Apr 17, 2014

I think I just closed it by accident while trying to comment.

On Thu, Apr 17, 2014 at 10:36 AM, JP Simard notifications@gh.neting.ccwrote:

Any particular reason why this PR is now closed?


Reply to this email directly or view it on GitHubhttps://github.com//pull/190#issuecomment-40740871
.

@alazier alazier reopened this Apr 17, 2014
@alazier
Copy link
Contributor

alazier commented Apr 17, 2014

When accidentally closing this, I was trying to say that even once we have merge rules in place, there will still be a delay for other threads/contexts trying to access the data which doesn't seem correct to me. Do we really want to force a mandatory delay before transactions on the main thread complete? I think it would be much preferable not to if possible, and to at a minimum give users of the apis a mechanism for manually indicating that they are done so that they can proceed without delay if desired.

I agree this issue wont effect this example once we have merging in place, as both the write transaction and read transaction would be a single transaction. In fact you could achieve the same thing right now without waiting for a notification by putting the call to insertRowsAtIndexPaths inside your original writeTransaction. If that does work I think that would be preferable, as it does not incur the delay for the ui to update after changing the table. You can see the delay right now by changing the background color when the edit is made and changing it back when the notification is received.

@jpsim
Copy link
Contributor Author

jpsim commented Apr 17, 2014

@alazier calling insertRowsAtIndexPaths: inside the write block won't help in this case because it still calls numberOfRowsInSection: which is using the out of date readContext to find its rowCount.

I agree that the ability to force a transaction commit would be nice, but the cases in which it would be useful would probably be quite few. If you're writing on the main thread, which is the only time transactions are automated at run-loop intervals, odds are you can wait until the next commit before your background transactions are aware of the change.

@alazier
Copy link
Contributor

alazier commented Apr 17, 2014

I think you are underestimating the frequency people will run into issues. Imagine the simple case of adding something to a table on the ui thread, and then wanting to dispatch an operation that operates on what was inserted in the background. The operation could be a network requests, file operation, anything really. Any time someone wants to do this they will run into this issue, and will have to be aware of this inherent delay and dispatch_after accordingly. I can't imagine this not causing confusion down the road and think we should be looking for a more elegant solution to identifying the 'end' of an implicit transaction.

jpsim added 11 commits April 17, 2014 14:19
- keep a strong reference to self.table property and lazily instantiate it
- align ='s
- added a checked Bool column to RLMDemoTable
- now using tableWithName:asTableClass: method on TDBSmartContext
- now displays a checkmark on even row numbers (backed by the checked BOOL column)
* master: (84 commits)
  libtightdb-objc -> librealm-objc
  build.sh: renamed more local bash variables from tightdb_* to realm_*
  build.sh: renamed tightdb_config_dbg_cmd to realm_config_dbg_cmd
  build.sh: renamed tightdb_config_cmd to realm_config_cmd
  build.sh & project.mk: renamed TIGHTDB_LDFLAGS_* to REALM_LDFLAGS_*
  build.sh & project.mk: renamed TIGHTDB_CFLAGS_* to REALM_CFLAGS_*
  build.sh: changed TIGHTDB_VERSION to REALM_VERSION
  renamed common_funcs from tightdb_* to realm_*
  Replaced TIGHTDB_IPHONE_CORE_LIB with REALM_IPHONE_CORE_LIB since it's not referenced in either TightDB Core or Docs
  minor style change: align ='s
  Renamed TIGHTDB_OBJC_HOME and TIGHTDB_CONFIG to REALM_* in build.sh and README's
  Replaced usage of TightDB with Realm in README & build.sh error messages
  Renamed everything in TightDbObjcDyn to RealmObjcDyn
  Updated context_ref.yaml
  Updated dyn_query_ref.yaml
  Updated dyn_table_ref.yaml
  Updated dyn_view_ref.yaml
  Updated reference.yaml
  Updated transaction_ref.yaml
  Updated typed_query_ref.yaml
  ...

Conflicts:
	examples/Makefile
	examples/TightdbTutorial.xcodeproj/project.pbxproj
	examples/iOSTutorial/AppDelegate.m
* master:
  removed fiel's hardcoded objc path from RefDocExamples's FRAMEWORK_SEARCH_PATHS
* master:
  Fixed issue when running table_view.m tests on a machine with non-english locale
@jpsim jpsim mentioned this pull request Apr 21, 2014
jpsim added 2 commits April 21, 2014 12:44
* master:
  Fixed issue stopping unit tests to run from the RealmObjcDyn Xcode project
@jpsim
Copy link
Contributor Author

jpsim commented Apr 21, 2014

This should be ready for merge, please review if you get a chance!


- (void)query {
// @@Example: query @@
RLMRow *row = [self.table find:[NSPredicate predicateWithFormat:@"checked = %@", @YES]];
Copy link
Contributor

Choose a reason for hiding this comment

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

We should really support varargs on find and where. Then you could write it like this:

RLMRow *row = [self.table find:@"checked = %@", @YES];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, maybe a findWithPredicateFormat: method.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll add this to the asana ticket with the other predicate improvements.
Should be pretty easy to do.

On Mon, Apr 21, 2014 at 4:52 PM, JP Simard notifications@github.com wrote:

In examples/RLMDemo/RLMDemo/RLMTableViewController.m:

  • }
    +}

+#pragma mark - Tutorial Examples
+
+- (void)iteration {

  • // @@example: iteration @@
  • for (RLMDemoTableRow *row in self.table) {
  •    NSLog(@"%@ is %@", row.title, row.checked ? @"checked" : @"unchecked");
    
  • }
  • // @@endexample@@
    +}

+- (void)query {

  • // @@example: query @@
  • RLMRow *row = [self.table find:[NSPredicate predicateWithFormat:@"checked = %@", @yES]];

Agreed, maybe a findWithPredicateFormat: method.


Reply to this email directly or view it on GitHubhttps://github.com//pull/190/files#r11832391
.

jpsim added 2 commits April 22, 2014 17:40
* master: (35 commits)
  Simplified iOSTutorial Xcode project and beautified simple_search example
  fixed more issues with iOSTutorial
  Fixed block method signature in iOSTutorial
  Added documentation for objectAtIndexedSubscript: and objectForKeyedSubscript:
  no need to remove .realm.lock file
  - RefDocExamples: changed "com.realm" company ID to "io.realm" - Re-added commented YAML removed in previous commit - Fixed issues with iOSTutorial (even though it's being replaced)
  Updated documentation for insertRow:atIndex: and renamed setRow:atIndex: to updateRow:atIndex:
  Renamed methods to make it more clear that an update operation is being performed.
  Updated unit tests and setter methods to be consistent with an `update` model instead of a `set` model. We will need to refactor those methods in a separate PR.
  Added documentation for indexed subscript
  Added objc_dyn_table_set_object
  Added setRow:atIndex: documentation
  Added documentation for dyn_table_ref
  These unit tests seems to fail now and then due to floating point precision
  startsWith is implemented for binary/nsdata
  Changed previous rowCount to NSUInteger
  removed dynamic_table.mm from test target to see if ci passes
  Added keyed subscripting for Typed Tables
  Made lookup changes per Alex comments
  Re-added dynamic_table.mm
  ...

Conflicts:
	doc/tutorial/template.md
	examples/RLMDemo/RLMDemo/RLMDemo-Info.plist
	examples/TightdbTutorial.xcodeproj/project.pbxproj
	examples/TightdbTutorial.xcodeproj/xcshareddata/xcschemes/iOSTutorial.xcscheme
	examples/iOSTutorial/AppDelegate.m
* master: (37 commits)
  Minor tweak to comment
  specify that RLMRealm's can only be created on the main thread (unless you know what you're doing)
  Fixed issues with examples
  - renamed _hasParentContext to (!) _usesImplicitTransactions - +realmWithDefaultPersistence and +realmWithPersistenceToFile: throw an exception when called from a thread other than the main thread - -hasTableWithName:withTableClass: no longer creates a table - tests were added to confirm these changes stay permanent
  Fixed tutorial issues introduced by latest changes in master
  Updated tutorial
  updated ref docs for RLMRealm
  Fixed some more example issues
  Updated copyright
  Removed copyright
  Updated example files (as reported by Jenkins)
  Updated ref and release note
  Casting for picky test framework (only needed in older versions of Xcode)
  updated ImplicitTransactionsDemo
  Fixed typo where TDBDynamicTableTests should have been RLM*
  Use optional argument to request rollback in write transactions
  Moved setter for variable
  Removed optimize method from documentation
  Updated import to use optimize on RLMTableFast
  Removed optimize from RLMTable interface and moved it to RLMTableFast. Need to import RLMTableFast to test cases that use optimize method
  ...

Conflicts:
	examples/iOSTutorial/AppDelegate.m
@jpsim jpsim changed the title [WIP] New Tutorial New Tutorial Apr 23, 2014
@jpsim
Copy link
Contributor Author

jpsim commented Apr 23, 2014

This is now ready for merge.

@astigsen
Copy link
Contributor

+1

jpsim added a commit that referenced this pull request Apr 23, 2014
@jpsim jpsim merged commit 99d4c67 into master Apr 23, 2014
@jpsim jpsim deleted the jp-new-tutorial branch April 23, 2014 05:10
kishikawakatsumi pushed a commit that referenced this pull request Sep 20, 2016
Fixes to Jenkinsfile after adding a submodule
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants