7
\$\begingroup\$

About: Core Data model for test-taking iOS app
See also: Are UML diagram reviews on-topic for CodeReview? and Reviewing "design"

I feel the linked question is off-topic because it is about a design-review, rather than a code-review.

Key point:

Here is my Core Data model:

image of core data model

Overall, what do you think of my database schema? Is it sufficient to handle a simple test-taking app?

The implementation itself is not under review here, just the design.

To me, this is something you should take to Programmers.SE, were it not that they don't like doing design reviews there unless you come with some strict requirements and use-cases. See https://softwareengineering.meta.stackexchange.com/questions/6961/green-fields-blue-skys-and-the-white-board-what-is-too-broad for more about that.

The only code that's shown in the question is code for two classes which seems to have been auto-generated. The user comes with a question about the classes:

  • Also, are Test_Questions and User_Answers necessary?
    • I could, in theory, have a relationship directly between Test andQuestion? Test -->> Question. I would like to know which is better.

There's no problem with the classes at hand except that maybe they shouldn't exist. This again is a design question - implementation of requirements to a data model... and we don't even have the requirements.

Personally, I'd close this as off-topic, but @nhgrif disagrees. Thus meta.

His main argument, as I understand it, is that CoreData models are code-backed. That is, when you change the CoreData model, you change the code. Like this, a CoreData model represents the code.

\$\endgroup\$
  • \$\begingroup\$ I will post a more complete answer later, but it is crucial to note that CoreData isn't simply UML... So don't post an answer about CoreData as if it is just UML... \$\endgroup\$ – nhgrif Jan 28 '15 at 13:26
5
\$\begingroup\$

Three meta questions? All rolled in to one.... right...

  1. Is the question on-topic
  2. are design questions on topic
  3. should it go to programmers.

Underlying that, there's the question of whether it is a well-asked question, or not.

To make things more confusing, there's my personal opinion and also my moderator opinion.


Personal hat!

Is the question on-topic?

No, I don't believe it is. A code review is something that happens when the code is about to be committed, etc. The asker in this question says:

I'm building an iOS app for test-taking and I want to be sure of my model before proceeding.

To me that means: I have a design and I want to know if the design is good before I write the code that implements it.

Again, to me, that means that the code that should be reviewed, is not yet written. To me, it means the contents of the question are an expression of the design, not the implementation.

Are design reviews on topic?

No, they are not. Designs and specifications are very similar in their role for programmers. Your code is written to fulfill the requirements as dictated by the design/specification. Additionally, when performing the code review, the whole purpose of the code review is to ensure the code meets the design and specification goals.

You cannot perform an in-depth code review (other than style, best practice, and other superficial things) unless you know what the design is that the code implements.

Is it the role of Code Review to perform reviews of what your code should be doing? No. Code Review is for reviews of how well the code does what it's supposed to do.

Migrate to programmers?

I'm never sure about migrating to programmers. Its scope is hard to figure out, and I am not there yet. I err on the side of caution, and, unless crystal clear, I don't recommend migrating anything to Programmers. This question in particular is not likely going to do well there.

Is it a good question?

It's nice that the asker has taken considerable time to put together lots of details and background on the question, but my personal feeling is that the effort is wasted here on Code Review. No, it's a well presented question but it's asking the wrong thing.

Moderator hat :

Is the question on-topic?

Yes.

There is code, community consensus has indicated that interfaces are reviewable.

The design component of the question can simply be ignored, and a superficial review of the code is fine (code style, indenting, etc.).

Just because it is a poor question, does not make it off-topic.

Are design questions on topic

No, but this question is only a partial design review, it has code parts.

Should it be migrated to programmers?

Nope, this question is both too broad in its request since there are many possible answers, and also too narrow in it's scope. It does not apply to "all programmers", just this one.

Overall

I am tempted to close it as "too broad", though the current answer is very interesting.

Note the answer is:

However, the model fails to support the ability of a User to take multiple Tests. To fix that, User.testTaken should be replaced by User.testsTaken and made into a one-to-many relationship with Test.

Also, it seems a bit odd that a Test can only be taken by a single User, but if that's your requirement, then so be it.

Note the consequences of that last statement. Because the design is wrong, any code that uses that design is broken.

Additionally, because the question is asking for a model/design review, the answer needs to be qualified with: but if that's your requirement, then so be it.

There's too much guessing required when the design is not stable.

\$\endgroup\$
  • \$\begingroup\$ How do you feel (with your mod hat on) about it being auto-generated code? \$\endgroup\$ – RubberDuck Jan 29 '15 at 3:34
  • 1
    \$\begingroup\$ @RubberDuck - I have taken the position that the source code is not the text-code, but the CoreData 'structure'. The program is compiled from that. The best way to present that 'source' is as screenshots, etc. \$\endgroup\$ – rolfl Jan 29 '15 at 3:43
5
\$\begingroup\$

Disclaimer: This answer will not address the linked question specifically. Rather, it will offer some clarification on what CoreData is and isn't, and how this compares to things it might easily be confused with.


Before we talk about what CoreData is, let's talk about what CoreData isn't.

  • CoreData isn't a database. CoreData happens to use SQLite as a means for storing objects on disk, but it is not a database. It has a means of representing relationships between objects, and this may feel similar to a database schema (and is frequently referred to as a database schema), but the schema of the backing SQLite database doesn't necessarily directly match the relationships you've set up in your CoreData data model... at least not in the way you might expect if you're thinking of a traditional database.
  • CoreData isn't a UML. UMLs are to CoreData what comments are to executable code. While comments are a useful way of explaining what the code itself does, the comments aren't actually executable code. And the code that the comments are paired with may or may not actually do what the comments claim them to do. A UML is an attempt to document a program, lay out a map of its classes, etc., but the actual implementation may or may not actually match the UML. Moreover, you can change the UML without changing the code, and you can change the code without changing the UML. This is not what CoreData is. CoreData isn't documentation. When you create objects in CoreData, you're actually creating code.

Importantly, CoreData does not even exist outside of your Xcode project when you are writing Objective-C or Swift applications. (I am unaware whether or not you can use CoreData with other languages.)

When you create a new project in Xcode, on the absolute first screen, there is a check box allowing you the option to use CoreData:

enter image description here

And when you are using CoreData in your project, you will have a file with an .xcdatamodel extension in your Project Navigator:

enter image description here

And when you click on the file, your editor gives you Xcode's CoreData model editor. This is where the screenshot from the linked question comes from. There are multiple views within this editor.

It's important to note that by simply setting up entities within CoreData, you are writing code. This is the exact same as when you drag and drop a button onto a view in an interface builder. You're actually writing code when you do this.

For example, let's say I drag and drop in the visual interface builder and set up the following interface:

enter image description here

Well, like it or not, despite not using my keyboard (other than typing the text for the label/button), I've actually written several lines of code.

The code I wrote looks something like this:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<document type="com.apple.InterfaceBuilder3.CocoaTouch.Storyboard.XIB" version="3.0" toolsVersion="6254" systemVersion="14B25" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" useAutolayout="YES" useTraitCollections="YES" initialViewController="BYZ-38-t0r">
    <dependencies>
        <plugIn identifier="com.apple.InterfaceBuilder.IBCocoaTouchPlugin" version="6247"/>
    </dependencies>
    <scenes>
        <!--View Controller-->
        <scene sceneID="tne-QT-ifu">
            <objects>
                <viewController id="BYZ-38-t0r" customClass="ViewController" sceneMemberID="viewController">
                    <layoutGuides>
                        <viewControllerLayoutGuide type="top" id="y3c-jy-aDJ"/>
                        <viewControllerLayoutGuide type="bottom" id="wfy-db-euE"/>
                    </layoutGuides>
                    <view key="view" contentMode="scaleToFill" id="8bC-Xf-vdC">
                        <rect key="frame" x="0.0" y="0.0" width="600" height="600"/>
                        <autoresizingMask key="autoresizingMask" widthSizable="YES" heightSizable="YES"/>
                        <subviews>
                            <label opaque="NO" userInteractionEnabled="NO" contentMode="left" horizontalHuggingPriority="251" verticalHuggingPriority="251" fixedFrame="YES" text="Code Review is AWESOME!" textAlignment="center" lineBreakMode="tailTruncation" baselineAdjustment="alignBaselines" adjustsFontSizeToFit="NO" translatesAutoresizingMaskIntoConstraints="NO" id="gQP-SH-P1b">
                                <rect key="frame" x="16" y="20" width="568" height="59"/>
                                <fontDescription key="fontDescription" type="boldSystem" pointSize="24"/>
                                <color key="textColor" red="1" green="1" blue="1" alpha="1" colorSpace="custom" customColorSpace="calibratedRGB"/>
                                <nil key="highlightedColor"/>
                            </label>
                            <button opaque="NO" contentMode="scaleToFill" fixedFrame="YES" contentHorizontalAlignment="center" contentVerticalAlignment="center" buttonType="roundedRect" lineBreakMode="middleTruncation" translatesAutoresizingMaskIntoConstraints="NO" id="L5z-hi-FnJ">
                                <rect key="frame" x="225" y="158" width="150" height="60"/>
                                <color key="backgroundColor" red="0.50196081400000003" green="0.0" blue="1" alpha="1" colorSpace="calibratedRGB"/>
                                <fontDescription key="fontDescription" type="system" pointSize="19"/>
                                <state key="normal" title="This is a button!">
                                    <color key="titleColor" white="1" alpha="1" colorSpace="calibratedWhite"/>
                                    <color key="titleShadowColor" white="0.5" alpha="1" colorSpace="calibratedWhite"/>
                                </state>
                            </button>
                        </subviews>
                        <color key="backgroundColor" white="0.0" alpha="1" colorSpace="calibratedWhite"/>
                    </view>
                </viewController>
                <placeholder placeholderIdentifier="IBFirstResponder" id="dkx-z0-nzr" sceneMemberID="firstResponder"/>
            </objects>
        </scene>
    </scenes>
</document>

Furthermore, from Interface Builder, I can set up properties for classes. Dropped a button on that view controller? Now I can set up the view controller's backing class to have a property for that button as well as a method to handle the button being tapped.

I can do all this without using my keyboard.

Okay, so what's the point of me talking about interface builders?

With CoreData, we're doing the same thing!

Model. View. Controller.

There's three parts to this design paradigm, right? With Interface Builder, we design our view and hook it up to the controller.

CoreData is like a visual interface builder for the model part of MVC. When you create your CoreData model, you're actually making changes to your program--to your executable source code. The image included in the linked question isn't just a road-map to the application that is a nice visual representation of what the data model is supposed to look like. The image included in the linked question ACTUALLY IS THE IMPLEMENTATION OF THE DATA MODEL!


Here, I've used CodeData two set up two entities with three attributes each:

enter image description here

And a relationship is described from Entity1 to Entity2.

But this isn't just a visual representation of what I want to implement. These attributes actually have types:

enter image description here enter image description here

And now, without writing any other code, I can immediately start using these entities in my CoreData application:

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions {
    NSManagedObjectContext *context = [self managedObjectContext];
    NSManagedObject *entity1 = [NSEntityDescription insertNewObjectForEntityForName:@"Entity1" inManagedObjectContext:context];
    [entity1 setValue:@42 forKey:@"attribute2"];
    NSError *error;
    if (![context save:&error]) {
#if DEBUG
        NSLog(@"Error saving entity: %@", error.localizedDescription);
#endif
    }
    return YES;
}

I can load from and save to disk using CoreData. It's a simple as that, and CoreData is extraordinarily efficient at reading from and writing to disk, and because it is backed by SQLite, there are ways to effectively seek out specific Entities you've saved using CoreData.

But using CoreData in this manner is actually quite rare. What is more likely is to use Xcode and CoreData to generate classes based off of your CoreData entities:

Following this series of steps:

enter image description here enter image description here enter image description here

Will add the following files to your Xcode project:

enter image description here

And the generated files will look just like this:

Entity1.h

#import <Foundation/Foundation.h>
#import <CoreData/CoreData.h>

@class Entity2;

@interface Entity1 : NSManagedObject

@property (nonatomic, retain) NSData * attribute1;
@property (nonatomic, retain) NSNumber * attribute2;
@property (nonatomic, retain) NSNumber * attribute3;
@property (nonatomic, retain) Entity2 *relationship;

@end

Entity1.m

#import "Entity1.h"

@implementation Entity1

@dynamic attribute1;
@dynamic attribute2;
@dynamic attribute3;
@dynamic relationship;

@end

Entity2.h

#import <Foundation/Foundation.h>
#import <CoreData/CoreData.h>

@interface Entity2 : NSManagedObject

@property (nonatomic, retain) NSNumber * attribute1;
@property (nonatomic, retain) NSDate * attribute2;
@property (nonatomic, retain) NSString * attribute3;

@end

Entity2.m

#import "Entity2.h"

@implementation Entity2

@dynamic attribute1;
@dynamic attribute2;
@dynamic attribute3;

@end

And they will look like that every time. The only difference is that Xcode will give you the option between using NSNumber or primitive data types for particular properties (integer types, floating point types, booleans).

I don't need to see the .h or .m files that Xcode has generated for the user unless the user has then modified these files in some way (which is common).


So, now that you've scrolled passed the lesson on what CoreData is, you're ready for the meat of the answer...

Is the linked question on-topic? I don't know.

But we can review poorly named classes, poorly named properties, poorly named variables, etc. We can review variables that are excessive and unnecessary. We can review variables that should be of a different type. We can review information that might be missing. ("I see your person class has a firstName property, but no lastName property. You should probably include a lastName property.")

Moreover, let's say we had a class called Family and a class called Person. Family could have a Person property called Father, and a Person property called Mother, and three Person properties called OldestChild, MiddleChild, and YoungestChild. And in reviewing these two classes, we could point out that the Family class probably isn't well designed because different families have different configurations... varying number of children, step-parents, step-children, half-siblings, etc.

What's the difference between reviewing the data model that someone wrote by hand with their keyboard to map these Family and Person classes versus reviewing the data model that someone built with CoreData? And in answering this question, keep in mind everything we've already reviewed in this post: CoreData is not a database schema; CoreData is not a UML; CoreData is an actual data model implementation.

\$\endgroup\$
  • 1
    \$\begingroup\$ my problem with this is that it would make auto-generated code on topic. That seems problematic to me. \$\endgroup\$ – RubberDuck Jan 29 '15 at 3:32
  • \$\begingroup\$ Auto generated coredata code questions can be closed as a duplicate of a canonical "how to improve this autogenerated coredata code" question. Unless these questions come with a code related question, they're off topic. \$\endgroup\$ – Pimgd Jan 29 '15 at 8:27

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .