This is a fun little story. A few months ago I wrote the following function, due to a need to get the index of a specific occurrence of a character (exclusive mode, by which I mean position you are passing is not part of the search) within a stream (yes, stream). Basically its being used to do a small validation on every message that is coming in from an EventSource.
While writing the function, I didn't give much thought. I wrote 3 unit tests, I then wrote the function and didn't give much thought.
private getNthIndex(text: string, delimiter: string, occurrence: number, position: number):number {
while (occurrence-- && position++ < text.length) {
position= text.indexOf(delimiter, position);
if (position < 0) break;
}
return position;
}
Then a fellow team member had a ticket to change something around that area. When running into that function, they told me that it was entirely confusing and it needed a bit of a thought to understand it properly, and whether if I mind, if it got refactored.
It has been nearly 9 months since I wrote that, and because it did took me a couple of minutes to remember why I did it like that, I agreed that if a more readable version is possible, obviously I would welcome it.
So they came up with the following
private getNthIndex(text: string, delimiter: string, occurrence: number, position: number):number {
for (let i = 0; i < occurrence; i++) {
position= text.indexOf(delimiter, position+ 1);
if (position < 0 || position>= text.length) break;
}
return position;
}
In my eyes, they are nearly identical and of the same level of readability. There are only two small changes the while
loop is now a for
loop, and fact that the counter goes up instead of going down.
So, just for the fun of it, I post it on the dev channel
to ask about opinions and weirdly I realised that some developers are really suspicious of while loops and that for
loops are easier to read than while
loops.
I lost 6 to 1 🎉