georgejkaye / delay-repay

Tool to calculate Delay Repay refunds
1 stars 1 forks source link

Significant delay when adding tickets #2

Closed FrozenDude101 closed 2 months ago

FrozenDude101 commented 2 months ago

pun intended

Steps to reproduce:

  1. Add a ticket
  2. Change the date/time, either expected or actual
  3. Cannot add another ticket, or remove the added ticket

Sometimes it adds the ticket after a significant delay.

georgejkaye commented 2 months ago

Wonder why this is happening

georgejkaye commented 2 months ago

The tickets are stored in a map so maybe there's a key clash, but I don't know why that would happen

FrozenDude101 commented 2 months ago

Why are the tickets stored in a map, it would make more sense to be an array

georgejkaye commented 2 months ago

So they can be updated in constant time

FrozenDude101 commented 2 months ago

This is premature optimisation at its finest

georgejkaye commented 2 months ago

It's future proofing

FrozenDude101 commented 2 months ago

Actually, its just incorrect logic, surely when the time chages all the tickets need to be updated? So its linear anyway, as you need to go through them all, and iterating over an array is still linear

georgejkaye commented 2 months ago

The tickets just store a price and whether they're single/return, changing the time shouldn't affect them

FrozenDude101 commented 2 months ago

I meant for updating the delay repay Maps have marginal improvements here with adding/removing tickets, but lose out in generally representing the data how it is used But this whole conversation is pointless anyway for your users (me) use case

georgejkaye commented 2 months ago

I use Array.from a lot anyway to convert it to an array so I might actually be making it worse performant

georgejkaye commented 2 months ago

image I don't think it has anything to do with the map

georgejkaye commented 2 months ago

Fixed by 678ef5d, having the functions in the useEffect dependencies (which is what I thought the React warning was telling me to but maybe not) caused useEffect to call infinitely