Refactoring/externalized html

[ziflex]

No description provided.

[codecov]

Codecov Report

Merging #234 into master will decrease coverage by 0.3%.
The diff coverage is 12%.

@@           Coverage Diff            @@
##           master    #234     +/-   ##
========================================
- Coverage    59.2%   58.9%   -0.3%     
========================================
  Files         181     179      -2     
  Lines        5234    5262     +28     
========================================
- Hits         3098    3097      -1     
- Misses       1791    1820     +29     
  Partials      345     345

[3timeslazy]

maybe switch-case would be better?

[ziflex]

Maybe :)

[3timeslazy]

if input.Type() returns types.Object, can we get ok == false here?

[3timeslazy]

also, we can switch input.(type), not input.Type()

It will looks like:

switch value := input.(type) {
	case Boolean,
		Int,
		Float,
		String,
		DateTime:

		return NewArrayWith(value), nil
	case *Array:
		return value.Copy(), nil
	case *Object:
		arr := NewArray(int(value.Length()))

		value.ForEach(func(value core.Value, key string) bool {
			arr.Push(value)

			return true
		})

		return value, nil
	case core.Iterable:
		iterator, err := value.Iterate(ctx)

		if err != nil {
			return None, err
		}

		arr := NewArray(10)

		for {
			val, _, err := iterator.Next(ctx)

			if err != nil {
				return None, err
			}

			if val == None {
				break
			}

			arr.Push(val)
		}

		return arr, nil
	default:
		return NewArray(0), nil
	}

[ziflex]

Valid point. But if a value has type of type.Object but isn't values.Object it means that someone messed up with types.
The reason why I created Type interface is that I wanted to avoid type assertion as much as I could because of its possible (in some cases) negative impact on performance.

[ziflex]

But ideally, we need to write benchmarks to see the impact. It seems in recent version of Go, it has been improved.

[3timeslazy]

I don't think this will be main performance problem, but OK :)

[3timeslazy]

Referring to #8 (comment), we should return only new instances. Think we should use Clone instead of Copy.

It can shoot here:

ferret/pkg/stdlib/types/to_array.go

Lines 16 to 24 in ba8f373

	func ToArray(ctx context.Context, args ...core.Value) (core.Value, error) {
	err := core.ValidateArgs(args, 1, 1)
	if err != nil {
	return values.None, err
	}
	return values.ToArray(ctx, args[0])
	}

[ziflex]

Well, I do not think it's valid here. We create a new array with the same content. I think it's an intended behavior, isn't it?