Skip to content

Modifying segment parsing to handle repetition with elements. #57

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jkamal000 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Modifying segment parsing to handle repetition with elements. #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 27 additions & 12 deletions lib/Segment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { Delimiters, FormattedSegment } from './types.js';

export class Segment {
#delimiters: Delimiters;
#parsed: string[][];
#parsed: string[][][];
#name: string;

/**
Expand Down Expand Up @@ -30,13 +30,26 @@ export class Segment {
get formatted(): FormattedSegment {
const formatted: FormattedSegment = { name: this.#name };

this.#parsed.forEach((element, elementIndex) => {
this.#parsed.forEach((elements, elementsIdx) => {
Copy link
Copy Markdown
Owner

@tastypackets tastypackets Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid renaming stuff in the PR?

// First item in parsed string is the segment value, everything else is components
formatted[`${elementIndex + 1}`] = element.shift() ?? '';
formatted[`${elementsIdx + 1}`] = elements.at(0)?.at(0) ?? '';
Copy link
Copy Markdown
Owner

@tastypackets tastypackets Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might be able to remove the logic around adjusting the index if we pull out the first element, maybe something like:

      const [firstElement, ...subsequentElements] = elements;
      formatted[elementKey] = firstElement?.[0] ?? '';

const repeats = elements.length > 1;

// Add components
element.forEach((component, componentIndex) => {
formatted[`${elementIndex + 1}-${componentIndex + 1}`] = component;
elements.forEach((element, repeatIdx) => {
const multiComponent = element.length > 1;
element.forEach((component, componentIdx) => {
// skipping first element
if (componentIdx == 0 && repeatIdx == 0) return;
// because we skipped first element we need to decrement index
// avoiding shifting because modifies internal list
Comment on lines +41 to +44
Copy link
Copy Markdown
Owner

@tastypackets tastypackets Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like these might be AI comments, could we remove them?

if (repeatIdx == 0) componentIdx -= 1;
const repeatString = repeats ? `-${repeatIdx + 1}` : '';
let key = `${elementsIdx + 1}`;
if (repeats || multiComponent) {
key = `${key}-${componentIdx + 1}${repeatString}`;
}
formatted[key] = component;
});
});
});

Expand All @@ -55,16 +68,18 @@ export class Segment {
/**
* Processes an element and formats it
* @param element The string inside the element
* @returns A nested array of segments & components
* @returns A nested array of segments & repeated elements & components
*/
processElement(element: string): string[] {
processElement(element: string): string[][] {
// If this is ISA we don't want to split on component
if (this.#name === 'ISA') {
return [Segment.cleanString(element)];
return [[Segment.cleanString(element)]];
} else {
return element
.split(this.#delimiters.component)
.map((string) => Segment.cleanString(string));
return element.split(this.#delimiters.repetition).map((string) => {
return string
.split(this.#delimiters.component)
.map((string) => Segment.cleanString(string));
});
}
}

Expand Down
57 changes: 54 additions & 3 deletions test/segment.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,25 @@ describe('Segment', () => {
it('Should return an array of strings, split by the component delimiter', () => {
//TODO: Remove initial segment when type checking is added
const mySegment = new Segment('AMT*AU*34.25', delimiters);
expect(mySegment.processElement('AU')).toStrictEqual(['AU']);
expect(mySegment.processElement('AU')).toStrictEqual([['AU']]);
});
it('If ISA component is left in tact, since it is an actual element', () => {
//TODO: Remove initial segment when type checking is added
const isa =
'ISA*00* *00* *ZZ*EMEDNYBAT *ZZ*ETIN *100101*1000*^*00501*006000600*0*T*:';
const mySegment = new Segment(isa, delimiters);
expect(mySegment.processElement(':')).toStrictEqual([':']);
expect(mySegment.processElement(':')).toStrictEqual([[':']]);
});
it('Should handle cases where elements are repeated', () => {
const ras = 'RAS*34.25*1234567890*12345:0^67890:1';
const mySegment = new Segment(ras, delimiters);
expect(mySegment.processElement('1234567890')).toStrictEqual([
['1234567890'],
]);
expect(mySegment.processElement('12345:0^67890:1')).toStrictEqual([
['12345', '0'],
['67890', '1'],
]);
});
});

Expand All @@ -50,7 +61,7 @@ describe('Segment', () => {
describe('#constructor()', () => {
const mySegment = new Segment('AMT*AU*34.25', delimiters);
it('Should parse passed raw element string', () => {
expect(mySegment.parsed).toStrictEqual([['AU'], ['34.25']]);
expect(mySegment.parsed).toStrictEqual([[['AU']], [['34.25']]]);
});

it('Should parse out the name of the element', () => {
Expand All @@ -67,5 +78,45 @@ describe('Segment', () => {
2: '34.25',
});
});
it('Should add repeat index to end if repeated element', () => {
const ras = 'RAS*34.25*1234567890*12345:0^67890:1';
const segment = new Segment(ras, delimiters);
expect(segment.formatted).toStrictEqual({
name: 'RAS',
1: '34.25',
2: '1234567890',
3: '12345',
'3-1-1': '0',
'3-1-2': '67890',
'3-2-2': '1',
});
});
it('Should add repeat index to if repeated and first is single component', () => {
const ras = 'RAS*34.25*1234567890*abcde^12345:0^67890:1';
const segment = new Segment(ras, delimiters);
expect(segment.formatted).toStrictEqual({
name: 'RAS',
1: '34.25',
2: '1234567890',
3: 'abcde',
'3-1-2': '12345',
'3-2-2': '0',
'3-1-3': '67890',
'3-2-3': '1',
Comment on lines +102 to +105
Copy link
Copy Markdown
Owner

@tastypackets tastypackets Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the next composite element end with 3? Should this reset to 1? I think it's going to make it impossible/hard to track where a repeat fits into the overall segment.

Copy link
Copy Markdown
Author

@jkamal000 jkamal000 Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if you still have notifications open for this (I forgot about this pr as I was jumping between projects).
The last digit in the composite index was originally intended to keep track of the order in which the definitions occurred.

Suppose we had RAS*subCompA0:subCompA1:subCompA2^subCompB0:subCompB1subCompB2^subCompC0:subCompC1:subCompC2
In this scenario I originally intended for the last digit in the key to represent the index of the repeated element within its own "list." The idea was for a way to tell that repeated elements go A then B then C etc.

The meaning of the key was intended to be elementIdx-subElementIdx-repetitionIdx
A key like 3-2-4 has the following meaning:

  • 3 indicates it is the 3rd component of the RAS segment
  • 2 indicates the value is the second sub component (the same way it was originally parsed)
  • 4 indicates that this is a part of the 4th repetition of the 3rd component. The third component can have a repeat of 15 so I used the last value to determine its order (which is 4, where we use 1-indexing)

The output was expected like

{
  name: "RAS",
  "1": "subCompA0",           // keeping with original scheme
  "1-1-1": "subCompA1",       // sub component 1 of repetition 1 of main component 1
  "1-2-1": "subCompA2",       // sub component 2 of repetition 1 of main component 1
  "1-1-2": "subCompB0",       // sub component 1 of repetition 2 of main component 1
  "1-2-2": "subCompB1",       // sub component 2 of repetition 2 of main component 1
  "1-3-2": "subCompB2",       // sub component 3 of repetition 2 of main component 1
  // ... rest
}

I agree that there may be a little confusion with putting the index of the repetition at the end instead of using a key likeelementIdx-repetitionIdx-subElementIdx however such a key would cause old systems, where the second number is the sub element index, to break.

});
});
it('Should format normal multi component elements', () => {
const ras = 'RAS*34.25*1234567890*12345:0:67890:1';
const segment = new Segment(ras, delimiters);
expect(segment.formatted).toStrictEqual({
name: 'RAS',
1: '34.25',
2: '1234567890',
3: '12345',
'3-1': '0',
'3-2': '67890',
'3-3': '1',
});
});
});
});