ronantakizawa/codereview-bench huggingface dataset:
Repository: realm/realm-swift
File: Realm/RLMArrayLinkView.mm (Objective-C++)

Code before:

- (void)replaceObjectAtIndex:(NSUInteger)index withObject:(RLMObject *)object {
    RLMLinkViewArrayValidateInWriteTransaction(self);

    RLMValidateObjectClass(object, self.objectClassName);
    if (index >= _backingLinkView->size()) {
        @throw [NSException exceptionWithName:@"RLMException"
                                       reason:@"Trying to replace object at invalid index" userInfo:nil];
    }
    if (object.realm != self.realm) {
        [self.realm addObject:object];
    }
    _backingLinkView->set(index, object->_row.get_index());
}

- (NSString *)JSONString {
    RLMLinkViewArrayValidateAttached(self);

    @throw [NSException exceptionWithName:@"RLMNotImplementedException"
                                   reason:@"Not yet implemented" userInfo:nil];
}

- (NSUInteger)indexOfObject:(RLMObject *)object {
    RLMLinkViewArrayValidateAttached(self);

    if (object->_row.get_table() != &_backingLinkView->get_parent()) {
        @throw [NSException exceptionWithName:@"RLMException"
                                       reason:@"Object type does not match RLMArray" userInfo:nil];
    }

    size_t object_ndx = object->_row.get_index();
    size_t result = _backingLinkView->find_by_source_ndx(object_ndx);
    if (result == tightdb::not_found) {
        return NSNotFound;
    }

    return result;
}

@end





Review comment (performance):
Why not:

```
 if(![object.RLMObjectSchema.className isEqual:_objectClassName])
```

Would be easier to understand read/understand albeit slower


Code after:

- (void)replaceObjectAtIndex:(NSUInteger)index withObject:(RLMObject *)object {
    RLMLinkViewArrayValidateInWriteTransaction(self);

    RLMValidateObjectClass(object, self.objectClassName);
    if (index >= _backingLinkView->size()) {
        @throw [NSException exceptionWithName:@"RLMException"
                                       reason:@"Trying to replace object at invalid index" userInfo:nil];
    }
    if (object.realm != self.realm) {
        [self.realm addObject:object];
    }
    _backingLinkView->set(index, object->_row.get_index());
}

- (NSString *)JSONString {
    RLMLinkViewArrayValidateAttached(self);

    @throw [NSException exceptionWithName:@"RLMNotImplementedException"
                                   reason:@"Not yet implemented" userInfo:nil];
}

- (NSUInteger)indexOfObject:(RLMObject *)object {
    RLMLinkViewArrayValidateAttached(self);

    if (object->_row.get_table() != &_backingLinkView->get_parent()) {
        @throw [NSException exceptionWithName:@"RLMException"
                                       reason:@"Object type does not match RLMArray"
                                     userInfo:nil];
    }

    size_t object_ndx = object->_row.get_index();
    size_t result = _backingLinkView->find(object_ndx);
    if (result == tightdb::not_found) {
        return NSNotFound;
    }

    return result;
}

@end
Repository: realm/realm-swift
File: build.sh (Shell)

Code before:
    xc "-project $PROJECT $1"
}

######################################
# Input Validation
######################################

if [ "$#" -eq 0 -o "$#" -gt 2 ]; then
    usage
    exit 1
fi

######################################
# Variables
######################################

# Xcode sets this variable - set to current directory if running standalone
if [ -z "$SRCROOT" ]; then
    SRCROOT="$(pwd)"
fi

download_core() {
    echo "Downloading dependency: core ${REALM_CORE_VERSION}"
    curl -L -s "http://static.realm.io/downloads/core/realm-core-${REALM_CORE_VERSION}.zip" -o "/tmp/core-${REALM_CORE_VERSION}.zip" || exit 1
    (
        cd /tmp
        unzip "/tmp/core-${REALM_CORE_VERSION}.zip" || exit 1
        mv core core-${REALM_CORE_VERSION}
        rm -f "/tmp/core-${REALM_CORE_VERSION}.zip" || exit 1
    )
    mv /tmp/core-${REALM_CORE_VERSION} .
    ln -s core-${REALM_CORE_VERSION} core
}

COMMAND="$1"
XCMODE="$2"
: ${XCMODE:=xcodebuild} # must be one of: xcodebuild (default), xcpretty, xctool


case "$COMMAND" in

    ######################################
    # Clean
    ######################################
    "clean")
        xcrealm "-scheme iOS -configuration Debug -sdk iphonesimulator clean" || exit 1
        xcrealm "-scheme iOS -configuration Release -sdk iphonesimulator clean" || exit 1
        xcrealm "-scheme OSX -configuration Debug clean" || exit 1
        xcrealm "-scheme OSX -configuration Release clean" || exit 1
        exit 0
        ;;

Review comment (refactor):
Let's leave the `exit 1` in there for now, it doesn't hurt to have them in duplicates. We can clean this up after.


Code after:
}

######################################
# Input Validation
######################################

if [ "$#" -eq 0 -o "$#" -gt 2 ]; then
    usage
    exit 1
fi

######################################
# Variables
######################################

# Xcode sets this variable - set to current directory if running standalone
if [ -z "$SRCROOT" ]; then
    SRCROOT="$(pwd)"
fi

download_core() {
    echo "Downloading dependency: core ${REALM_CORE_VERSION}"
    curl -L -s "http://static.realm.io/downloads/core/realm-core-${REALM_CORE_VERSION}.zip" -o "/tmp/core-${REALM_CORE_VERSION}.zip" || exit 1
    (
        cd /tmp
        unzip "/tmp/core-${REALM_CORE_VERSION}.zip" || exit 1
        mv core core-${REALM_CORE_VERSION}
        rm -f "/tmp/core-${REALM_CORE_VERSION}.zip" || exit 1
    )
    mv /tmp/core-${REALM_CORE_VERSION} .
    ln -s core-${REALM_CORE_VERSION} core
}

COMMAND="$1"
XCMODE="$2"
: ${XCMODE:=xcodebuild} # must be one of: xcodebuild (default), xcpretty, xctool


case "$COMMAND" in

    ######################################
    # Clean
    ######################################
    "clean")
        xcrealm "-scheme iOS -configuration Debug -sdk iphonesimulator clean" || exit 1
        xcrealm "-scheme iOS -configuration Release -sdk iphonesimulator clean" || exit 1
        xcrealm "-scheme OSX -configuration Debug clean" || exit 1
        xcrealm "-scheme OSX -configuration Release clean" || exit 1
        exit 0
        ;;
Repository: realm/realm-swift
File: tools/RealmBrowser/RealmBrowser/Classes/RLMClassNode.m (Objective-C)

Code before:
    return NSNotFound;
}

- (NSView *)cellViewForTableView:(NSTableView *)tableView
{
    RLMSidebarTableCellView *result = [tableView makeViewWithIdentifier:@"MainCell" owner:self];

    result.textField.stringValue = self.name;
    result.button.title = [NSString stringWithFormat:@"%lu", (unsigned long)[self instanceCount]];
    [[result.button cell] setHighlightsBy:0];
    result.button.hidden = NO;
    result.imageView.image = nil;
    
    return result;
}

#pragma mark - Public methods

- (RLMObjectNode *)displayChildObject:(RLMObject *)object
{
    displaysQuery = NO;
    
    RLMObjectNode *objectNode = [[RLMObjectNode alloc] initWithObject:object realm:self.realm];
    objectNode.parentNode = self;
    
    if (displayedObjects.count == 0) {
        [displayedObjects addObject:objectNode];
    }
    else {
        [displayedObjects replaceObjectAtIndex:0
                                   withObject:objectNode];
    }

    return objectNode;
}

- (RLMArrayNode *)displayChildArrayFromQuery:(NSString *)searchText result:(RLMArray *)result
{
    displaysQuery = YES;

    RLMArrayNode *arrayNode = [[RLMArrayNode alloc] initWithQuery:searchText result:result andParent:self];

    if (displayedArrays.count == 0) {
        [displayedArrays addObject:arrayNode];
    }
    else {
        [displayedArrays replaceObjectAtIndex:0 withObject:arrayNode];
    }

    return arrayNode;
}

Review comment (refactor):
This is only moved up to public methods, it used to be a private property but in order to eliminate the helper methods, we needed a way to access the underlying array from outside.


Code after:
    return NSNotFound;
}

- (NSView *)cellViewForTableView:(NSTableView *)tableView
{
    RLMSidebarTableCellView *result = [tableView makeViewWithIdentifier:@"MainCell" owner:self];

    result.textField.stringValue = self.name;
    result.button.title = [NSString stringWithFormat:@"%lu", (unsigned long)[self instanceCount]];
    [[result.button cell] setHighlightsBy:0];
    result.button.hidden = NO;
    result.imageView.image = nil;
    
    return result;
}

- (RLMArray *)allObjects
{
    if (!_allObjects) {
        _allObjects = [self.realm allObjects:self.schema.className];
    }
    
    return _allObjects;
}

#pragma mark - Public methods

- (RLMObjectNode *)displayChildObject:(RLMObject *)object
{
    displaysQuery = NO;
    
    RLMObjectNode *objectNode = [[RLMObjectNode alloc] initWithObject:object realm:self.realm];
    objectNode.parentNode = self;
    
    if (displayedObjects.count == 0) {
        [displayedObjects addObject:objectNode];
    }
    else {
        [displayedObjects replaceObjectAtIndex:0 withObject:objectNode];
    }

    return objectNode;
}

- (RLMArrayNode *)displayChildArrayFromQuery:(NSString *)searchText result:(RLMArray *)result
{
    displaysQuery = YES;

    RLMArrayNode *arrayNode = [[RLMArrayNode alloc] initWithQuery:searchText result:result andParent:self];

    if (displayedArrays.count == 0) {
Repository: realm/realm-swift
File: Realm/RLMObjectStore.hpp (C++)

Code before:
//
// Adding, Removing, Getting Objects
//

// add an object to the given realm
void RLMAddObjectToRealm(RLMObject *object, RLMRealm *realm, RLMSetFlag options = 0);

// delete an object from its realm
void RLMDeleteObjectFromRealm(RLMObject *object);

// get objects of a given class
RLMResults *RLMGetObjects(RLMRealm *realm, NSString *objectClassName, NSPredicate *predicate);

// get an object with the given primary key
id RLMGetObject(RLMRealm *realm, NSString *objectClassName, id key);

// create object from array or dictionary
RLMObject *RLMCreateObjectInRealmWithValue(RLMRealm *realm, NSString *className, id value, RLMSetFlag options = 0);


//
// Accessor Creation
//

// Create accessors
RLMObject *RLMCreateObjectAccessor(__unsafe_unretained RLMRealm *realm,
                                   __unsafe_unretained NSString *objectClassName,
                                   NSUInteger index);
RLMObject *RLMCreateObjectAccessor(__unsafe_unretained RLMRealm *realm,
                                   __unsafe_unretained RLMObjectSchema *objectSchema,
                                   NSUInteger index);


Review comment (suggestion):
I don't think these should make much of a difference as these methods should only be called during schema generation.


Code after:

// add an object to the given realm
void RLMAddObjectToRealm(RLMObject *object, RLMRealm *realm, RLMSetFlag options = 0);

// delete an object from its realm
void RLMDeleteObjectFromRealm(RLMObject *object, RLMRealm *realm);

// deletes all objects from a realm
void RLMDeleteAllObjectsFromRealm(RLMRealm *realm);

// get objects of a given class
RLMResults *RLMGetObjects(RLMRealm *realm, NSString *objectClassName, NSPredicate *predicate);

// get an object with the given primary key
id RLMGetObject(RLMRealm *realm, NSString *objectClassName, id key);

// create object from array or dictionary
RLMObject *RLMCreateObjectInRealmWithValue(RLMRealm *realm, NSString *className, id value, RLMSetFlag options = 0);


//
// Accessor Creation
//

// Create accessors
RLMObject *RLMCreateObjectAccessor(__unsafe_unretained RLMRealm *realm,
                                   __unsafe_unretained NSString *objectClassName,
                                   NSUInteger index);
RLMObject *RLMCreateObjectAccessor(__unsafe_unretained RLMRealm *realm,
                                   __unsafe_unretained RLMObjectSchema *objectSchema,
                                   NSUInteger index);
Repository: realm/realm-swift
File: tools/RealmBrowser/RealmBrowser/Classes/Realm_Private.h (C/C++)

Code before:
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
////////////////////////////////////////////////////////////////////////////

#import <Realm/Realm.h>

#ifndef RealmBrowser_Realm_Private_h
#define RealmBrowser_Realm_Private_h

@interface RLMRealm (Dynamic)

+ (instancetype)realmWithPath:(NSString *)path
                     readOnly:(BOOL)readonly
                     inMemory:(BOOL)inMemory
                      dynamic:(BOOL)dynamic
                       schema:(RLMSchema *)customSchema
                        error:(NSError **)outError;

- (RLMResults *)allObjects:(NSString *)className;

- (RLMArray *)objects:(NSString *)className where:(NSString *)predicateFormat, ...;

@end

@interface RLMArray (Private)

- (instancetype)initWithObjectClassName:(NSString *)objectClassName;

@end

#endif


Review comment (suggestion):
This should return an RLMResults, right? It looks like I had it written that way too, I wonder why xcode didn't catch that. EDIT: It was because this is based on the master, which I hadn't edited for RLMResults support except in allObjects. My working copy used the correct signature, which is why I hadn't seen any warning.


Code after:
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
////////////////////////////////////////////////////////////////////////////

#import <Realm/Realm.h>

@interface RLMRealm (Dynamic)

+ (instancetype)realmWithPath:(NSString *)path
                     readOnly:(BOOL)readonly
                     inMemory:(BOOL)inMemory
                      dynamic:(BOOL)dynamic
                       schema:(RLMSchema *)customSchema
                        error:(NSError **)outError;

- (RLMResults *)allObjects:(NSString *)className;

- (RLMArray *)objects:(NSString *)className where:(NSString *)predicateFormat, ...;

@end

@interface RLMArray (Private)

- (instancetype)initWithObjectClassName:(NSString *)objectClassName;

@end
Repository: realm/realm-swift
File: Realm/RLMObjectBase.h (C/C++)

Code before:
// limitations under the License.
//
////////////////////////////////////////////////////////////////////////////

#import <Realm/RLMConstants.h>
#import <Realm/RLMProperty.h>

RLM_HEADER_AUDIT_BEGIN(nullability, sendability)

/// :nodoc:
@interface RLMObjectBase : NSObject

@property (nonatomic, readonly, getter = isInvalidated) BOOL invalidated;

- (instancetype)init NS_DESIGNATED_INITIALIZER;

+ (NSString *)className;

// Returns whether the class is included in the default set of classes managed by a Realm.
+ (BOOL)shouldIncludeInDefaultSchema;

+ (nullable NSString *)_realmObjectName;
+ (nullable NSDictionary<NSString *, NSString *> *)_realmColumnNames;

/// Allow client code to generate properties (ie. via Swift Macros)
+ (nullable NSArray<RLMProperty *> *)_customRealmProperties;

@end

RLM_HEADER_AUDIT_END(nullability, sendability)


Review comment (performance):
You'll see here I've added a static method directly to the base class here. In [the original code by](https://github.com/realm/realm-swift/pull/8261/files) @tgoyne, he defined a Swift Protocol and then added conformance via the Macro. I couldn't work with that solution for two main reasons:

1. Supporting Object subclassing was really tricky. There is no way to know from an Extension Macro if the class subclasses from a type that already conforms to this protocol - it just sees the exact syntax of this local type. So the Macro would naively add the conformance, then generate a compile time error that the type already conforms.
2. The purpose of this Macro is to improve runtime performance. In profiling my own use case, I noticed that adding this casting of the generic Base class to this Swift protocol type was not free. It added ~100ms to the schema generation. Which is almost equivalent to the time taken to generate all the actual properties for all my Objects. By defining the method on all Base Objects avoided this cast and that cost. 

![image](https://github.com/realm/realm-swift/assets/5361118/4f82b449-f95b-4b26-9aaa-d094b074b1f5)

Code after:
// limitations under the License.
//
////////////////////////////////////////////////////////////////////////////

#import <Realm/RLMConstants.h>

RLM_HEADER_AUDIT_BEGIN(nullability, sendability)

/// :nodoc:
@interface RLMObjectBase : NSObject

@property (nonatomic, readonly, getter = isInvalidated) BOOL invalidated;

- (instancetype)init NS_DESIGNATED_INITIALIZER;

+ (NSString *)className;

// Returns whether the class is included in the default set of classes managed by a Realm.
+ (BOOL)shouldIncludeInDefaultSchema;

+ (nullable NSString *)_realmObjectName;
+ (nullable NSDictionary<NSString *, NSString *> *)_realmColumnNames;

@end

RLM_HEADER_AUDIT_END(nullability, sendability)
Repository: recharts/recharts
File: src/component/Label.tsx (TypeScript)

Code before:
    return null;
  }

  if (label === true) {
    return <Label key="label-implicit" viewBox={viewBox} />;
  }

  if (isNumOrStr(label)) {
    return <Label key="label-implicit" viewBox={viewBox} value={label} />;
  }

  if (isValidElement(label)) {
    if (label.type === Label) {
      return cloneElement<LabelProps>(label, { key: 'label-implicit', viewBox });
    }

    return <Label key="label-implicit" content={label} viewBox={viewBox} />;
  }

  if (isLabelContentAFunction(label)) {
    return <Label key="label-implicit" content={label} viewBox={viewBox} />;
  }

  if (label && typeof label === 'object') {
    return <Label viewBox={viewBox} {...label} key="label-implicit" />;
  }

  return null;
};

const renderCallByParent = (
  parentProps: { children?: ReactNode; label?: unknown },
  viewBox?: ViewBox,
  checkPropsLabel = true,
): ReactElement[] | null => {
  if (!parentProps || (!parentProps.children && checkPropsLabel && !parentProps.label)) {
    return null;
  }
  const { children } = parentProps;
  const parentViewBox = parseViewBox(parentProps);

  const explicitChildren = findAllByType(children, Label).map((child, index) => {
    return cloneElement(child, {
      viewBox: viewBox || parentViewBox,
      // eslint-disable-next-line react/no-array-index-key
      key: `label-${index}`,
    });
  });

  if (!checkPropsLabel) {
    return explicitChildren;

Review comment (suggestion):
when `label` is an HTML or SVG element `label` gets passed to `content` and then `content` is cloned with `props`. `props` includes `labelRef` which isn't a valid prop in the DOM

Code after:
  }

  return {};
};

const parseLabel = (label: unknown, viewBox: ViewBox, labelRef?: React.RefObject<Element>) => {
  if (!label) {
    return null;
  }

  const commonProps = { viewBox, labelRef };

  if (label === true) {
    return <Label key="label-implicit" {...commonProps} />;
  }

  if (isNumOrStr(label)) {
    return <Label key="label-implicit" value={label} {...commonProps} />;
  }

  if (isValidElement(label)) {
    if (label.type === Label) {
      return cloneElement<LabelProps>(label, { key: 'label-implicit', ...commonProps });
    }

    return <Label key="label-implicit" content={label} {...commonProps} />;
  }

  if (isLabelContentAFunction(label)) {
    return <Label key="label-implicit" content={label} {...commonProps} />;
  }

  if (label && typeof label === 'object') {
    return <Label {...label} key="label-implicit" {...commonProps} />;
  }

  return null;
};

const renderCallByParent = (
  parentProps: {
    children?: ReactNode;
    label?: unknown;
    labelRef?: React.RefObject<Element>;
  },
  viewBox?: ViewBox,
  checkPropsLabel = true,
): ReactElement[] | null => {
  if (!parentProps || (!parentProps.children && checkPropsLabel && !parentProps.label)) {
    return null;
  }
Repository: recharts/recharts
File: src/util/BarUtils.tsx (TypeScript)

Code before:
  return (
    <Shape
      shapeType="rectangle"
      propTransformer={typeguardBarRectangleProps}
      activeClassName="recharts-active-bar"
      {...props}
    />
  );
}

export type MinPointSize = number | ((value: number | undefined | null, index: number) => number);

/**
 * Safely gets minPointSize from from the minPointSize prop if it is a function
 * @param minPointSize minPointSize as passed to the Bar component
 * @param defaultValue default minPointSize
 * @returns minPointSize
 */
export const minPointSizeCallback =
  (minPointSize: MinPointSize, defaultValue = 0) =>
  (value: unknown, index: number): number => {
    if (typeof minPointSize === 'number') return minPointSize;
    const isValueNumber = typeof value === 'number' || typeof value === 'undefined' || value === null;
    if (isValueNumber) {
      return minPointSize(value as number | undefined | null, index);
    }

    invariant(
      isValueNumber,
      `minPointSize callback function received a value with type of ${typeof value}. Currently only numbers are supported.`,
    );
    return defaultValue;
  };


Review comment (refactor):
The change in the condition should also allow you to remove this I think? Prefer not to cast if possible 

Code after:
export function BarRectangle(props: BarRectangleProps) {
  return (
    <Shape
      shapeType="rectangle"
      propTransformer={typeguardBarRectangleProps}
      activeClassName="recharts-active-bar"
      {...props}
    />
  );
}

export type MinPointSize = number | ((value: number | undefined | null, index: number) => number);

/**
 * Safely gets minPointSize from from the minPointSize prop if it is a function
 * @param minPointSize minPointSize as passed to the Bar component
 * @param defaultValue default minPointSize
 * @returns minPointSize
 */
export const minPointSizeCallback =
  (minPointSize: MinPointSize, defaultValue = 0) =>
  (value: unknown, index: number): number => {
    if (typeof minPointSize === 'number') return minPointSize;
    const isValueNumberOrNil = isNumber(value) || isNullish(value);
    if (isValueNumberOrNil) {
      return minPointSize(value as number | undefined | null, index);
    }

    invariant(
      isValueNumberOrNil,
      `minPointSize callback function received a value with type of ${typeof value}. Currently only numbers or null/undefined are supported.`,
    );
    return defaultValue;
  };