• Jeff (unregistered)

    I think I need to go for a lie down after that

  • Anonymous (unregistered)

    Obvious fix: force entry IDs to be prime numbers and replace sums by products.

  • (nodebb) in reply to Anonymous

    No, use GUIDs. If GUIDs are unique, then their sum is also always unique.

  • DQ (unregistered) in reply to Mr. TA

    I think the original programmer followed the same logic:
    If two numbers are unique, then their sum must also be unique

  • Zach (unregistered) in reply to Mr. TA

    deep, very deep

  • King (unregistered)

    Dare I say that todays WTF is placement of braces? Yes? No?

  • Church (unregistered) in reply to King

    No, I don't think you can say that. While not my style, it's common enough. At least I feel like I see it quite frequently on GitHub projects and various other places online.

  • Dude (unregistered) in reply to King

    That's normal brace placement for C#, which I'd be pretty confident in saying this code is (.OrderBy being Linq).

  • Pedro (unregistered)

    closings.OrderBy(c => $"{Math.Min(c.EntryId,c.RefEntryId)}.{Math.Max(c.EntryId,c.RefEntryId)}")

    ... might have worked. Or a faster solution if the IDs are not 64bit longs: closings.OrderBy(c => ((long)Math.Min(c.EntryId,c.RefEntryId) << 32) + Math.Max(c.EntryId,c.RefEntryId))

  • Anony Moose (unregistered) in reply to Mr. TA

    Um, not sure if being sarcastic, but... No.

  • Zach (unregistered) in reply to Anony Moose

    Take a wild guess. He was probably referencing tongue-in-cheek yesterday's WTF

  • Some Guy (unregistered)

    Easy Reader Version: // the rest of the Easy Reader Version omitted

  • I dunno LOL ¯\(°_o)/¯ (unregistered) in reply to Mr. TA

    Well, not exactly, if you treat them as the big integers they really are, but we know that these idiots love to do everything with strings (to the point where they will commonly .ToString() a string), and the concatenation of two GUIDs is definitely unique!

  • (nodebb)

    In order to maintain this code, you gotta have a pair.

  • Kleyguerth (github)

    No easy reader version? How can I understand the article without an easy reader version?

  • (nodebb)

    I think the problem is not one of being able to master data structures, but one of being able to master 'sums', specifically, that there are multiple pairs of numbers that can add up to the same sum. Something that most people realize in elementary school - usually early on - or before. That someone would get to the position of a developer and not understand (or not realize/remember) a basic fact involving grade school math is dismaying, but not surprising - sadly.

  • sizer99 (google) in reply to Anonymous

    I know it was facetious, but what the heck... if this guy doesn't realize that multiple pairs of numbers can add up to the same sum he's the kind of guy who would think any odd number is a prime.

  • Zach (unregistered) in reply to Anonymous

    I'm going to pretend you were serious. Forcing ID's to be prime is outside the scope of the function, so it isn't possible and would require ID's to get very high very fast. Instead we can combine their approach with your approach - combine addition with multiplication (12 and 13 would give us 25_156, whereas 11 and 14 would give us 24_154) so that they all would indeed be unique. Don't actually do this in real life, but this could be a simple patch to guarantee uniqueness

  • Anonymous (unregistered) in reply to King

    Egyptian brackets are TRWTF, they are simply less readable than C#'s aligned brackets. Egyptian brackets were fine 30 years ago, when only 15 lines of code could fit on a screen at once. But nowadays, you're just wasting readability for no reason.

  • (nodebb) in reply to Zach

    Not that fast, really; 2^31-1 is already the 105097565th prime, giving that many possible "Entry"s and still allowing products of pairs to fit in a 64-bit int.

  • Dave (unregistered)

    Couldn't you just concatenate the IDs with a separator 11->14 becomes "11-14" while 10->15 becomes "10-15"

  • (nodebb)

    The choice of grouping key aside, TRWTF is that this guy knows enough Linq to use OrderBy, but writes an entire method when a single GroupBy call would suffice.

  • Pedro (unregistered)

    Moderation takes way too long on this forum. I posted an alternate solution up there.... by the time it shows, no one cares anymore.

    This one might go into moderation too... who knows, seems to be random.

  • P (unregistered)

    Let's post a ton of alternate solutions! Figuring out which solution is the most WTF is left as an exercise to the reader:

    var last = default(Closing);
foreach (var c in closings.OrderBy(c => c.EntryId + c.RefEntryId).ThenBy(c => Math.Abs(c.EntryId - c.RefEntryId)).ThenBy(c => c.EntryId))
{
    if (c.EntryId == last?.RefEntryId) yield return last;
    last = c;
}

    foreach (var c in closings.Where(c => closing.Any(d => c.EntryId == d.RefEntryId &&  d.EntryId == c.RefEntryId)) yield return c;

    var closingLookup = closings.ToLookup(c => c.EntryId);
foreach (var c in closings.Where(c => closingLookup[c.RefEntryId].Any(d => d.RefEntryId == c.EntryId))) yield return c;
  • P (unregistered)

    Also, TRWTF here is that the "the rest of the logic omitted" comment cannot be the original comment: otherwise since a closingGroup that is never assigned to is being yielded, either it return nothing or it keeps returning the same Closing that is unrelated to the entire closing list.

    Checkmate, Remy!

  • Jim J (unregistered) in reply to P

    What about

    return closings.GroupBy(e => new { minId = Min(e.EntryId, e.RefEntryId), maxId = Max(e.EntryId, e.RefEntryId) }).AsEnumerable();

  • Anon (unregistered)

    Who said IDs are numbers? This code works "fine" if they are fixed length strings.

  • Dude (unregistered) in reply to Anon

    Except it doesn't accomplish the original goal, which is to match objects that are reciprocal of each other (eg, 1 <-> 2 would not match 2 <-> 1)

  • println::hello (unregistered)

    my try:

        List<Closing> allClosings = new ArrayList<>();
    allClosings.add(new Closing(2, 6));
    allClosings.add(new Closing(1, 3));
    allClosings.add(new Closing(6, 2));
    allClosings.add(new Closing(5, 7));
    allClosings.add(new Closing(10, 11));
    allClosings.add(new Closing(3, 1));
    allClosings.add(new Closing(9, 1));
    allClosings.add(new Closing(1, 9));

    Map<Set<Integer>, List<Closing>> res = allClosings.stream().collect(Collectors.groupingBy(closing -> Set.of(closing.getEnityA(), closing.getEnityB())));
    System.out.println(res.size());
    System.out.println(res);

    or:

        Map<ClosingPair, List<Closing>> results = allClosings.stream().collect(Collectors.groupingBy(ClosingPair::fromClosing));
    System.out.println(results.size());
    System.out.println(results);

    public class ClosingPair { private Set<Integer> entites = new HashSet<>();

    static ClosingPair fromClosing(Closing closing) {
    return new ClosingPair(closing.getEnityA(), closing.getEnityB());
}

ClosingPair(int enityA, int enityB) {
    entites.add(enityA);
    entites.add(enityB);
}

@Override
public boolean equals(Object o) {
    if (this == o) return true;
    if (o == null || getClass() != o.getClass()) return false;
    ClosingPair that = (ClosingPair) o;
    return Objects.equals(entites, that.entites);
}

@Override
public int hashCode() {
    return Objects.hash(entites);
}

@Override
public String toString() {
    return "ClosingPair{" +
            entites.toString() +
            '}';
}

    }

  • markm (unregistered)

    If 'EntryId' and 'ExitId' are unsigned 32-bit integers, define 'key' as a 64 bit unsigned integer.

    key = (min(c.EntryId,c.ExitId)<<32) + max(c.EntryId,c.ExitId)

    Now sort by the keys.

    This makes 'key' the largest member of the pair appended after the smallest member of the pair. E.g., 20->21 and 21->20 have the same key, but no other combination can have that key. A comparison between even huge integers is very quick, so a properly chosen sort algorithm should be quick.

  • println::hello (unregistered)

    Isn't a solution using a map much cleaner? Min/max integer comparison was my first though, too. Sure it is fast, but it seems like a hack. And it breaks if you once decide to change your EntryId and ExitId to 64bit integers. (without changing your logic)

  • ikariw (unregistered)

    I assumed TRWTF was the missing apostrophe in "its"

  • (nodebb) in reply to Anonymous

    Actually I think you can get away with using Fibonacci numbers.

  • UUIDs (unregistered) in reply to Mr. TA

    Only Microshaft uses GUIDs, the rest of us use UUIDs.

Leave a comment on “The Pair of All Sums”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article