Repository: jerryscript-project/jerryscript
File: jerry-core/ecma/builtin-objects/ecma-builtin-helpers.c (C)

Code before:

  ecma_string_t *index_str_p = ecma_new_ecma_string_from_length (index);
  ecma_value_t ret_value = ecma_builtin_helper_def_prop (obj_p, index_str_p, value, opts);
  ecma_deref_ecma_string (index_str_p);

  return ret_value;
} /* ecma_builtin_helper_def_prop_by_index */

/**
 * Helper function for at() functions.
 *
 * See also:
 *          ECMA-262 Stage 3 Draft Relative Indexing Method proposal 3. 4. 5. 6.
 *
 * Used by:
 *         - The Array.prototype.at routine.
 *         - The String.prototype.at routine.
 *         - The TypedArray.prototype.at routine.
 *
 * @return ECMA_VALUE_ERROR - on conversion error
 *         ECMA_VALUE_UNDEFINED - if the requested index is not exist
 *         ECMA_VALUE_EMPTY - otherwise
 */
ecma_value_t
ecma_builtin_helper_check_index (ecma_value_t index, /**< index argument */
                                 ecma_number_t *len) /**< [out] object's length */
{
  ecma_number_t relative_index;
  ecma_value_t conversion_result = ecma_op_to_integer (index, &relative_index);

   /* 4. */
  if (ECMA_IS_VALUE_ERROR (conversion_result))
  {
    return ECMA_VALUE_ERROR;
  }

  /* 5. 6. */
  ecma_number_t k;

  if (relative_index >= 0)
  {
    k = relative_index;
  }
  else
  {
    k = (*len + relative_index);
  }

  /* 7. */
  if (k < 0 || k >= (*len))
  {

Review comment (refactor):
Is this really the length? It seems to me that this is used as input length, but it is also used to return the calculated index. Which is weird. It should be better to have a third argument which returns the calculated index, eg.:
```c
ecma_builtin_helper_check_index(ecma_value_t index,
                                ecma_length_t length,
                                ecma_number_t *out_index)
```
This way the input length can be of `ecma_length_t` type.

Also:
* Should we return `ecma_number_t` at all? I think we could return `ecma_length_t` typed value. (This needs a check on how there resulting index is used in most cases).
* This method does not "check" but "calulate"-s the required index. Maybe we could use the `ecma_builtin_helper_calculate_index` name?

Code after:

  ecma_string_t *index_str_p = ecma_new_ecma_string_from_length (index);
  ecma_value_t ret_value = ecma_builtin_helper_def_prop (obj_p, index_str_p, value, opts);
  ecma_deref_ecma_string (index_str_p);

  return ret_value;
} /* ecma_builtin_helper_def_prop_by_index */

/**
 * Helper function for at() functions.
 *
 * See also:
 *          ECMA-262 Stage 3 Draft Relative Indexing Method proposal 3. 4. 5. 6.
 *
 * Used by:
 *         - The Array.prototype.at routine.
 *         - The String.prototype.at routine.
 *         - The TypedArray.prototype.at routine.
 *
 * @return ECMA_VALUE_ERROR - on conversion error
 *         ECMA_VALUE_UNDEFINED - if the requested index is not exist
 *         ECMA_VALUE_EMPTY - otherwise
 */
ecma_value_t
ecma_builtin_helper_calculate_index (ecma_value_t index, /**< relative index argument */
                                     ecma_length_t length, /**< object's length */
                                     ecma_length_t *out_index) /**< calculated index */
{
  JERRY_ASSERT (out_index != NULL);

  ecma_number_t relative_index;
  ecma_value_t conversion_result = ecma_op_to_integer (index, &relative_index);

   /* 4. */
  if (ECMA_IS_VALUE_ERROR (conversion_result))
  {
    return ECMA_VALUE_ERROR;
  }

  /* 5. 6. */
  ecma_number_t k;

  if (relative_index >= 0)
  {
    k = relative_index;
  }
  else
  {
    k = ((ecma_number_t) length + relative_index);
  }