ronantakizawa/codereview-bench huggingface dataset:
Repository: react-hook-form/react-hook-form
File: src/useForm.ts (TypeScript)

Code before:

    control._formState.isReady = true;

    return sub;
  }, [control]);

  React.useEffect(
    () => control._disableForm(props.disabled),
    [control, props.disabled],
  );

  React.useEffect(() => {
    if (props.mode) {
      control._options.mode = props.mode;
    }
    if (props.reValidateMode) {
      control._options.reValidateMode = props.reValidateMode;
    }
    if (props.errors && !isEmptyObject(props.errors)) {
      control._setErrors(props.errors);
    }
  }, [control, props.errors, props.mode, props.reValidateMode]);

  React.useEffect(() => {
    props.shouldUnregister &&
      control._subjects.state.next({
        values: control._getWatch(),
      });
  }, [control, props.shouldUnregister]);

  React.useEffect(() => {
    if (control._proxyFormState.isDirty) {
      const isDirty = control._getDirty();
      if (isDirty !== formState.isDirty) {
        control._subjects.state.next({
          isDirty,
        });
      }
    }
  }, [control, formState.isDirty]);

  React.useEffect(() => {
    if (props.values && !deepEqual(props.values, _values.current)) {
      control._reset(props.values, control._options.resetOptions);
      _values.current = props.values;
      updateFormState((state) => ({ ...state }));
    } else {
      control._resetDefaultValues();
    }
  }, [control, props.values]);


Review comment (suggestion):
Since `props.errors` is an object, wouldn't that make `useEffect()` run at every render since the object reference is not the same? This is probably not related to the PR itself per se since it's already present before, but might be worth looking into.

Code after:

    control._formState.isReady = true;

    return sub;
  }, [control]);

  React.useEffect(
    () => control._disableForm(props.disabled),
    [control, props.disabled],
  );

  React.useEffect(() => {
    if (props.mode) {
      control._options.mode = props.mode;
    }
    if (props.reValidateMode) {
      control._options.reValidateMode = props.reValidateMode;
    }
  }, [control, props.mode, props.reValidateMode]);

  React.useEffect(() => {
    if (props.errors && !isEmptyObject(props.errors)) {
      control._setErrors(props.errors);
      control._focusError();
    }
  }, [control, props.errors]);

  React.useEffect(() => {
    props.shouldUnregister &&
      control._subjects.state.next({
        values: control._getWatch(),
      });
  }, [control, props.shouldUnregister]);

  React.useEffect(() => {
    if (control._proxyFormState.isDirty) {
      const isDirty = control._getDirty();
      if (isDirty !== formState.isDirty) {
        control._subjects.state.next({
          isDirty,
        });
      }
    }
  }, [control, formState.isDirty]);

  React.useEffect(() => {
    if (props.values && !deepEqual(props.values, _values.current)) {
      control._reset(props.values, control._options.resetOptions);
      _values.current = props.values;
      updateFormState((state) => ({ ...state }));
    } else {
Repository: react-hook-form/react-hook-form
File: src/utils/get.ts (TypeScript)

Code before:
import isKey from './isKey';
import isNullOrUndefined from './isNullOrUndefined';
import isObject from './isObject';
import isUndefined from './isUndefined';
import stringToPath from './stringToPath';

export default <T>(
  object: T,
  path?: string | null,
  defaultValue?: unknown,
): any => {
  if (!path || !isObject(object)) {
    return defaultValue;
  }

  const tempPath = isKey(path) ? [path] : stringToPath(path);
  const result = tempPath.reduce(
    (result, key) =>
      isNullOrUndefined(result) ? result : result[key as keyof {}],
    object,
  );

  return isUndefined(result) || result === object
    ? isUndefined(object[path as keyof T])
      ? defaultValue
      : object[path as keyof T]
    : result;
};


Review comment (suggestion):
Save an extra variable, can just keep temPath inline.

`(isKey(path) ? [path] : stringToPath(path)).reduce`

Code after:
import isKey from './isKey';
import isNullOrUndefined from './isNullOrUndefined';
import isObject from './isObject';
import isUndefined from './isUndefined';
import stringToPath from './stringToPath';

export default <T>(
  object: T,
  path?: string | null,
  defaultValue?: unknown,
): any => {
  if (!path || !isObject(object)) {
    return defaultValue;
  }

  const result = (isKey(path) ? [path] : stringToPath(path)).reduce(
    (result, key) =>
      isNullOrUndefined(result) ? result : result[key as keyof {}],
    object,
  );

  return isUndefined(result) || result === object
    ? isUndefined(object[path as keyof T])
      ? defaultValue
      : object[path as keyof T]
    : result;
};
Repository: react-hook-form/react-hook-form
File: src/logic/generateId.ts (TypeScript)

Code before:
export default () => {
  const d =
    typeof performance === 'undefined' ? Date.now() : performance.now() * 1000;

  return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, (c) => {
    const r = (Math.random() * 16 + d) % 16 | 0;

    return (c == 'x' ? r : (r & 0x3) | 0x8).toString(16);
  });
};


Review comment (security):
@bluebill1049 I think this is an awesome change, and I know this already got merged. But I just wanted to point out that [`crypto.randomUUID()`](https://developer.mozilla.org/en-US/docs/Web/API/Crypto/randomUUID) will only work in secure contexts. So it might be good to only execute this function [in that scenario](https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts#feature_detection).

If this isn't a concern, feel free to ignore. :)

Code after:
import isUndefined from '../utils/isUndefined';

export default () => {
  if (!isUndefined(crypto) && crypto.randomUUID) {
    return crypto.randomUUID();
  }

  const d =
    typeof performance === 'undefined' ? Date.now() : performance.now() * 1000;

  return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, (c) => {
    const r = (Math.random() * 16 + d) % 16 | 0;

    return (c == 'x' ? r : (r & 0x3) | 0x8).toString(16);
  });
};
Repository: react-hook-form/react-hook-form
File: src/useWatch.ts (TypeScript)

Code before:
      _prevName.current = name;
      refreshValue();
    }

    return control._subscribe({
      name,
      formState: {
        values: true,
      },
      exact,
      callback: (formState) => {
        refreshValue(formState.values);
      },
    });
  }, [control, exact, name, refreshValue]);

  React.useEffect(() => control._removeUnmounted());

  // If name or control changed for this render, synchronously reflect the
  // latest value so callers (like useController) see the correct value
  // immediately on the same render.

  // Optimize: Check control reference first before expensive deepEqual
  const controlChanged = _prevControl.current !== control;
  const nameChanged = !controlChanged && !deepEqual(_prevName.current, name);
  const shouldReturnImmediate = controlChanged || nameChanged;

  // Cache the computed output to avoid duplicate calls within the same render
  // We include shouldReturnImmediate in deps to ensure proper recomputation
  const computedOutput = React.useMemo(
    () => (!disabled && shouldReturnImmediate ? getCurrentOutput() : null),
    [disabled, shouldReturnImmediate, getCurrentOutput],
  );

  return computedOutput !== null ? computedOutput : value;
}


Review comment (suggestion):
I think we can bring this as part of the following `memo`, probably only need the controlChanged to be left out.

Code after:
      _prevName.current = name;
      refreshValue();
    }

    return control._subscribe({
      name,
      formState: {
        values: true,
      },
      exact,
      callback: (formState) => {
        refreshValue(formState.values);
      },
    });
  }, [control, exact, name, refreshValue]);

  React.useEffect(() => control._removeUnmounted());

  // If name or control changed for this render, synchronously reflect the
  // latest value so callers (like useController) see the correct value
  // immediately on the same render.

  // Optimize: Check control reference first before expensive deepEqual
  const controlChanged = _prevControl.current !== control;
  const prevName = _prevName.current;

  // Cache the computed output to avoid duplicate calls within the same render
  // We include shouldReturnImmediate in deps to ensure proper recomputation
  const computedOutput = React.useMemo(() => {
    if (disabled) {
      return null;
    }

    const nameChanged = !controlChanged && !deepEqual(prevName, name);
    const shouldReturnImmediate = controlChanged || nameChanged;

    return shouldReturnImmediate ? getCurrentOutput() : null;
  }, [disabled, controlChanged, name, prevName, getCurrentOutput]);

  return computedOutput !== null ? computedOutput : value;
}
Repository: react-hook-form/react-hook-form
File: src/useFieldArray.ts (TypeScript)

Code before:
    TKeyName,
    TTransformedValues
  >,
): UseFieldArrayReturn<TFieldValues, TFieldArrayName, TKeyName> {
  const methods = useFormContext();
  const {
    control = methods.control,
    name,
    keyName = 'id',
    shouldUnregister,
    rules,
  } = props;
  const [fields, setFields] = React.useState(control._getFieldArray(name));
  const ids = React.useRef<string[]>(
    control._getFieldArray(name).map(generateId),
  );

  const _actioned = React.useRef(false);

  control._names.array.add(name);

  rules &&
    (control as Control<TFieldValues, any, TTransformedValues>).register(
      name as FieldPath<TFieldValues>,
      rules as RegisterOptions<TFieldValues>,
    );

  useIsomorphicLayoutEffect(
    () =>
      control._subjects.array.subscribe({
        next: ({
          values,
          name: fieldArrayName,
        }: {
          values?: FieldValues;
          name?: InternalFieldName;
        }) => {
          if (fieldArrayName === name || !fieldArrayName) {
            const fieldValues = get(values, name);
            if (Array.isArray(fieldValues)) {
              setFields(fieldValues);
              ids.current = fieldValues.map(generateId);
            }
          }
        },
      }).unsubscribe,
    [control, name],
  );

  const updateValues = React.useCallback(
    <

Review comment (suggestion):
could we try to use rules with ref instead? would be good if we can still keep the memo.

```tsx
const rulesRef = useRef(rules)

reulsRef.current = rules
```

Code after:
    TKeyName,
    TTransformedValues
  >,
): UseFieldArrayReturn<TFieldValues, TFieldArrayName, TKeyName> {
  const methods = useFormContext();
  const {
    control = methods.control,
    name,
    keyName = 'id',
    shouldUnregister,
    rules,
  } = props;
  const [fields, setFields] = React.useState(control._getFieldArray(name));
  const ids = React.useRef<string[]>(
    control._getFieldArray(name).map(generateId),
  );

  const _actioned = React.useRef(false);

  control._names.array.add(name);

  React.useMemo(
    () =>
      rules &&
      fields.length >= 0 &&
      (control as Control<TFieldValues, any, TTransformedValues>).register(
        name as FieldPath<TFieldValues>,
        rules as RegisterOptions<TFieldValues>,
      ),
    [control, name, fields.length, rules],
  );

  useIsomorphicLayoutEffect(
    () =>
      control._subjects.array.subscribe({
        next: ({
          values,
          name: fieldArrayName,
        }: {
          values?: FieldValues;
          name?: InternalFieldName;
        }) => {
          if (fieldArrayName === name || !fieldArrayName) {
            const fieldValues = get(values, name);
            if (Array.isArray(fieldValues)) {
              setFields(fieldValues);
              ids.current = fieldValues.map(generateId);
            }
          }
        },
      }).unsubscribe,
Repository: realm/realm-swift
File: doc/ref/examples/ex_objc_table_typed_intro_with_many_comments.m (Objective-C)

Code before:
// @@Example: not_used @@

#import <tightdb/objc/table.h>
#import <tightdb/objc/tightdb.h>

// Defines a new table with two columns Name and Age. IMPORTANT: the column
// names MUST begin with a capital. This table definition triggers a macro
// defining the classes PeopleTable, PeopleTable_Query, PeopleTable_Curser
// and PeopleTable_View. The column types must be TightDB types (refer to
// the constructor TIGHTDB_TABLE_* in the documentation.

TIGHTDB_TABLE_2(PeopleTable,
                Name, String,
                Age, Int)

int main()
{
    @autoreleasepool {

        // Creates a new table of the type defined above.

        PeopleTable *table = [[PeopleTable alloc] init];

        // Adds rows to the table. Notice that the signtaure of the method for
        // adding rows includes the custom defined column nammes.

        [table addName:@"Mary" Age:14];
        [table addName:@"Joe" Age:17];
        [table addName:@"Jack" Age:22];
        [table addName:@"Paul" Age:33];
        [table addName:@"Simon" Age:16];

Review comment (suggestion):
Emacs users tend to like this (due to Meta-q) while users of other editors often find the leading `*` a bit difficult to work with :-)


Code after:
/* @@Example: not_used @@ */

#import <tightdb/objc/table.h>
#import <tightdb/objc/tightdb.h>

/* Defines a new table with two columns Name and Age. IMPORTANT: the column
 * names MUST begin with a capital. This table definition triggers a macro
 * defining the classes PeopleTable, PeopleTable_Query, PeopleTable_Curser
 * and PeopleTable_View. The column types must be TightDB types (refer to
 * the constructor TIGHTDB_TABLE_* in the documentation. */

TIGHTDB_TABLE_2(PeopleTable,
                Name, String,
                Age, Int)

int main()
{
    @autoreleasepool {

        /* Creates a new table of the type defined above. */

        PeopleTable *table = [[PeopleTable alloc] init];

        /* Adds rows to the table. Notice that the signtaure of the method for
         * adding rows includes the custom defined column nammes. */

        [table addName:@"Mary" Age:14];
        [table addName:@"Joe" Age:17];
        [table addName:@"Jack" Age:22];
        [table addName:@"Paul" Age:33];
        [table addName:@"Simon" Age:16];
Repository: realm/realm-swift
File: src/tightdb/objc/TDBTable.mm (Objective-C++)

Code before:
#import <tightdb/objc/TDBView_noinst.h>
#import <tightdb/objc/TDBQuery.h>
#import <tightdb/objc/TDBQuery_noinst.h>
#import <tightdb/objc/TDBRow.h>
#import <tightdb/objc/TDBDescriptor.h>
#import <tightdb/objc/TDBDescriptor_noinst.h>
#import <tightdb/objc/TDBColumnProxy.h>
#import <tightdb/objc/NSData+TDBGetBinaryData.h>

#include <tightdb/objc/util_noinst.hpp>

using namespace std;

@implementation TDBTable
{
    tightdb::TableRef m_table;
    id m_parent;
    BOOL m_read_only;
    TDBRow* m_tmp_row;
}



-(instancetype)init
{
    self = [super init];
    if (self) {
        m_read_only = NO;
        m_table = tightdb::Table::create(); // FIXME: May throw
    }
    return self;
}

-(instancetype)initWithColumns:(NSArray *)columns
{
    self = [super init];
    if (!self)
        return nil;

    m_read_only = NO;
    m_table = tightdb::Table::create(); // FIXME: May throw

    if (!set_columns(m_table, columns)) {
        m_table.reset();

        // Parsing the schema failed
        //TODO: More detailed error msg in exception
        NSException* exception = [NSException exceptionWithName:@"tightdb:invalid_columns"
                                                         reason:@"The supplied list of columns was invalid"
                                                       userInfo:[NSMutableDictionary dictionary]];
        [exception raise];

Review comment (suggestion):
instancetype makes it explicit that it returns an object of the same type as the class. With init it does also happen implicitly (clang overrides the type of id for init and alloc), but it should be good style to make it explicit.


Code after:
#import <tightdb/objc/TDBView_noinst.h>
#import <tightdb/objc/TDBQuery.h>
#import <tightdb/objc/TDBQuery_noinst.h>
#import <tightdb/objc/TDBRow.h>
#import <tightdb/objc/TDBDescriptor.h>
#import <tightdb/objc/TDBDescriptor_noinst.h>
#import <tightdb/objc/TDBColumnProxy.h>
#import <tightdb/objc/NSData+TDBGetBinaryData.h>
#import <tightdb/objc/PrivateTDB.h>


#include <tightdb/objc/util_noinst.hpp>

using namespace std;

@implementation TDBTable
{
    tightdb::TableRef m_table;
    id m_parent;
    BOOL m_read_only;
    TDBRow* m_tmp_row;
}



-(instancetype)init
{
    self = [super init];
    if (self) {
        m_read_only = NO;
        m_table = tightdb::Table::create(); // FIXME: May throw
    }
    return self;
}

-(instancetype)initWithColumns:(NSArray *)columns
{
    self = [super init];
    if (!self)
        return nil;

    m_read_only = NO;
    m_table = tightdb::Table::create(); // FIXME: May throw

    if (!set_columns(m_table, columns)) {
        m_table.reset();

        // Parsing the schema failed
        //TODO: More detailed error msg in exception
        NSException* exception = [NSException exceptionWithName:@"tightdb:invalid_columns"
                                                         reason:@"The supplied list of columns was invalid"
Repository: realm/realm-swift
File: examples/RealmTableViewExample/RealmTableViewExample/TableViewController.m (Objective-C)

Code before:
}

- (void)tableView:(UITableView *)tableView commitEditingStyle:(UITableViewCellEditingStyle)editingStyle forRowAtIndexPath:(NSIndexPath *)indexPath {
    if (editingStyle == UITableViewCellEditingStyleDelete) {
        [self.realm beginWriteTransaction];
        [self.table removeRowAtIndex:indexPath.row];
        [self.realm commitWriteTransaction];
    }
}

#pragma mark - Actions

- (void)add {
    [self.realm beginWriteTransaction];
    [self.table addRow:@[[self randomString], [self randomDate]]];
    [self.realm commitWriteTransaction];
}

- (void)deleteAll {
    [self.realm beginWriteTransaction];
    [self.table removeAllRows];
    [self.realm commitWriteTransaction];
}

#pragma - Helpers

- (NSString *)randomString {
    return [NSString stringWithFormat:@"Title %d", arc4random()];
}

- (NSDate *)randomDate {
    return [NSDate dateWithTimeIntervalSince1970:arc4random()];
}

@end


Review comment (performance):
Can we take a quick poll on this @Tightdb/ios? Background Add in table view example.

Yay/Nay?

Pros: demonstrates a very useful concept that 95% of developers will need to do
Cons: adds complexity to the app (navbar button, longpress gesture to delete all, blocks the other add method for short periods)


Code after:
    
    return cell;
}

- (void)tableView:(UITableView *)tableView commitEditingStyle:(UITableViewCellEditingStyle)editingStyle forRowAtIndexPath:(NSIndexPath *)indexPath {
    if (editingStyle == UITableViewCellEditingStyleDelete) {
        [self.realm beginWriteTransaction];
        [self.table removeRowAtIndex:indexPath.row];
        [self.realm commitWriteTransaction];
    }
}

#pragma mark - Actions

- (void)backgroundAdd {
    dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
    // Import many items in a background thread
    dispatch_async(queue, ^{
        // Get new realm and table since we are in a new thread
        RLMRealm *realm = [RLMRealm defaultRealm];
        RLMTable *table = [realm tableWithName:kTableName objectClass:[DemoObject class]];
        [realm beginWriteTransaction];
        for (NSInteger index = 0; index < 5; index++) {
            // Add row via dictionary. Order is ignored.
            [table addRow:@{@"title": [self randomString], @"date": [self randomDate]}];
        }
        [realm commitWriteTransaction];
    });
}

- (void)add {
    [self.realm beginWriteTransaction];
    [self.table addRow:@[[self randomString], [self randomDate]]];
    [self.realm commitWriteTransaction];
}

- (void)deleteAll {
    [self.realm beginWriteTransaction];
    [self.table removeAllRows];
    [self.realm commitWriteTransaction];
}

#pragma - Helpers

- (NSString *)randomString {
    return [NSString stringWithFormat:@"Title %d", arc4random()];
}

- (NSDate *)randomDate {
    return [NSDate dateWithTimeIntervalSince1970:arc4random()];
}
Repository: realm/realm-swift
File: examples/RealmTableViewExample/RealmTableViewExample/TableViewController.m (Objective-C)

Code before:
}

- (void)tableView:(UITableView *)tableView commitEditingStyle:(UITableViewCellEditingStyle)editingStyle forRowAtIndexPath:(NSIndexPath *)indexPath {
    if (editingStyle == UITableViewCellEditingStyleDelete) {
        [self.realm beginWriteTransaction];
        [self.table removeRowAtIndex:indexPath.row];
        [self.realm commitWriteTransaction];
    }
}

#pragma mark - Actions

- (void)add {
    [self.realm beginWriteTransaction];
    [self.table addRow:@[[self randomString], [self randomDate]]];
    [self.realm commitWriteTransaction];
}

- (void)deleteAll {
    [self.realm beginWriteTransaction];
    [self.table removeAllRows];
    [self.realm commitWriteTransaction];
}

#pragma - Helpers

- (NSString *)randomString {
    return [NSString stringWithFormat:@"Title %d", arc4random()];
}

- (NSDate *)randomDate {
    return [NSDate dateWithTimeIntervalSince1970:arc4random()];
}

@end


Review comment (suggestion):
Answer to the poll: definitely maybe.

My question is: how common is background updating in apps? If common, then our example should use it. Otherwise, don't complicate the source code.


Code after:
    
    return cell;
}

- (void)tableView:(UITableView *)tableView commitEditingStyle:(UITableViewCellEditingStyle)editingStyle forRowAtIndexPath:(NSIndexPath *)indexPath {
    if (editingStyle == UITableViewCellEditingStyleDelete) {
        [self.realm beginWriteTransaction];
        [self.table removeRowAtIndex:indexPath.row];
        [self.realm commitWriteTransaction];
    }
}

#pragma mark - Actions

- (void)backgroundAdd {
    dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
    // Import many items in a background thread
    dispatch_async(queue, ^{
        // Get new realm and table since we are in a new thread
        RLMRealm *realm = [RLMRealm defaultRealm];
        RLMTable *table = [realm tableWithName:kTableName objectClass:[DemoObject class]];
        [realm beginWriteTransaction];
        for (NSInteger index = 0; index < 5; index++) {
            // Add row via dictionary. Order is ignored.
            [table addRow:@{@"title": [self randomString], @"date": [self randomDate]}];
        }
        [realm commitWriteTransaction];
    });
}

- (void)add {
    [self.realm beginWriteTransaction];
    [self.table addRow:@[[self randomString], [self randomDate]]];
    [self.realm commitWriteTransaction];
}

- (void)deleteAll {
    [self.realm beginWriteTransaction];
    [self.table removeAllRows];
    [self.realm commitWriteTransaction];
}

#pragma - Helpers

- (NSString *)randomString {
    return [NSString stringWithFormat:@"Title %d", arc4random()];
}

- (NSDate *)randomDate {
    return [NSDate dateWithTimeIntervalSince1970:arc4random()];
}