beancount/beangulp

Is it safe to modify ledger entries in place?

dnicolodi opened this issue · 1 comments

Entries are named tuples and are thus immutable, but they contain mutable objects: lists and dictionaries. Is it advisable to modify these in place? The current deduplication code, for example, does this to add a __duplicate__ metadata field:

        mod_entries = []
        for entry in new_entries:
            if id(entry) in duplicate_set:
                marked_meta = entry.meta.copy()
                marked_meta[DUPLICATE_META] = True
                entry = entry._replace(meta=marked_meta)
            mod_entries.append(entry)

with requires copying the entries, recreating the entries list, and iterating the entries list one extra times.

The meta dictionary is however mutable, thus this could be written:

        for entry in new_entries:
            if id(entry) in duplicate_set:
                entry.meta[DUPLICATE_META] = True

and some more refactoring could get rid of the extra iteration.

This, of course, breaks if someone has been reusing the same dictionary for the metadata of more entries. Should this be considered an error (it is possible to check the refcount number of the dictionary to detect this) or should in-place modification of entries be avoided?

The code now assumes that it is safe to modify the entries in place. If someone has been reusing the same metadata dict they will quickly notice that things are off and identifying and fixing the issue is easy enough.