Simplify address.direction

[pkuczynski]

Clear and concise description of the problem

There are 3 separate functions now direction, cardinalDirection and ordinalDirection, which is not convenience to use. Also they depend on specific order of localisation, which is hard to verify when reviewing PR.

Suggested solution

Should we change the definition to look more like:

interface Directions {
  cardinal: string[]
  ordinal: string[] // or intercardinal ?
  secondaryIntercardinal?: string[]
}

type DirectionType = keyof Directions

And then both directions and directions_abbr should use this type?

Additionally, I think it makes sense to simplify the usage and change direction signature to:

direction ({ abbr: boolean, type: DirectionType, types: DirectionType[]})

// @examples
faker.address.direction() // 'north'
faker.address.direction({ abbr: true }) // 'N'
faker.address.direction({ type: 'ordinal' }) // 'north west'
faker.address.direction({ abbr: true, type: 'ordinal' }) // 'NW'
faker.address.direction({ abbr: true, types: ['ordinal', 'secondaryIntercardinal']  }) // 'NNW'

And then deprecate the other methods?

Alternative

We might also skip secondaryIntercardinal...

Additional context

No response

[Shinigami92]

The proposed signature confuses me a bit:
e.g. what will happen if you call faker.address.direction({ abbr: true, type: 'cardinal', types: ['ordinal'] })
I as a user calling this or reading this code would be totally confused what it could return

[pkuczynski]

Yeah, good point! How about direction ({ abbr: boolean, types: DirectionType[] }) or direction ({ abbr?: boolean, type?: DirectionType, except?: DirectionType[]})

[Shinigami92]

Both are better, lets discuss more on team meeting
But second proposed is also dangerous as you can call direction ({ type: ['ordinal'], except: ['ordinal'] }) and we would need to throw an error

[pkuczynski]

That's correct. But I guess more predictable. And unlikely someone would do it anyway. So we have the following proposals on the table, just to summarise:

direction ({ abbr?: boolean, type?: DirectionType | DirectionType[] })
direction ({ abbr?: boolean, types?: DirectionType[] })
direction ({ abbr?: boolean, type?: DirectionType, except?: DirectionType[]})

I think I like the first most.

[ST-DDT]

Team decision

Refactor our locales to match these:

interface Directions {
  cardinal: string[]
  ordinal: string[]
}

Merge the directions functions into a single method:

direction ({ abbr?: boolean = false, type?: DirectionType | DirectionType[] = ['cardinal', 'ordinal'] })

Throw if type is an empty array.

[wael-fadlallah]

Hello, I can take care of this one, please assign it to me

[pkuczynski]

We already discussed this with @hankucz and she will look at it. Thank you @wael-fadlallah

[hankucz]

Yeah, I'm already looking into it

[xDivisionByZerox]

I'm closing this as #2476 provides the suggested change. If you think this is a mistake, please reopen the issue or create a new one with additional, related information.

• refactor(locale)!: remove location data index-value-binding #2476