Repository: valkey-io/valkey
File: src/t_hash.c (C)

Code before:
     * hashes, where HGETDEL should respond with a series of null bulks. */
    robj *o = lookupKeyWrite(c->db, c->argv[1]);
    if (checkType(c, o, OBJ_HASH)) return;

    /* Reply with array of values */
    addReplyArrayLen(c, num_fields);
    for (i = fields_index; i < c->argc; i++) {
        addHashFieldToReply(c, o, c->argv[i]->ptr);
    }

    /* If hash doesn't exist, we're done as we have already replied with NULLs */
    if (o == NULL) return;

    /* Now delete the fields. */
    bool hash_volatile_items = hashTypeHasVolatileFields(o);
    for (i = fields_index; i < c->argc; i++) {
        if (hashTypeDelete(o, c->argv[i]->ptr)) {
            deleted++;
            if (hashTypeLength(o) == 0) {
                if (hash_volatile_items) dbUntrackKeyWithVolatileItems(c->db, o);
                dbDelete(c->db, c->argv[1]);
                keyremoved = true;
                break;
            }
        }
    }

    if (deleted) {
        if (!keyremoved && hash_volatile_items != hashTypeHasVolatileFields(o)) {
            dbUpdateObjectWithVolatileItemsTracking(c->db, o);
        }
        signalModifiedKey(c, c->db, c->argv[1]);
        notifyKeyspaceEvent(NOTIFY_HASH, "hgetdel", c->argv[1], c->db->id);
        if (keyremoved) notifyKeyspaceEvent(NOTIFY_GENERIC, "del", c->argv[1], c->db->id);
        server.dirty += deleted;
    }
}

void hlenCommand(client *c) {
    robj *o;

    if ((o = lookupKeyReadOrReply(c, c->argv[1], shared.czero)) == NULL || checkType(c, o, OBJ_HASH)) return;

    addReplyLongLong(c, hashTypeLength(o));
}

void hstrlenCommand(client *c) {
    robj *o;

    if ((o = lookupKeyReadOrReply(c, c->argv[1], shared.czero)) == NULL || checkType(c, o, OBJ_HASH)) return;
    addReplyLongLong(c, hashTypeGetValueLength(o, c->argv[2]->ptr));

Review comment (suggestion):
I think it is fine to avoid over optimization in favor of simplicity in this case.
I mean optimal implementation would be to "pop" the element from the hashtable take the value and free the memory (in case encoding is hashtable). I do think that we can improve by combining this loop with the following one eg (didn't test it :)):
```suggestion
    /* Reply with array of values */
    addReplyArrayLen(c, num_fields);
    for (i = fields_index; i < c->argc; i++) {
        addHashFieldToReply(c, o, c->argv[i]->ptr);
         /* If hash doesn't exist or the field is not found we just reply with NULL */
        if (o == NULL || !hashTypeDelete(o, c->argv[i]->ptr)) continue;
        deleted++;
        if (hashTypeLength(o) == 0) {
            if (hash_volatile_items) dbUntrackKeyWithVolatileItems(c->db, o);
            dbDelete(c->db, c->argv[1]);
            keyremoved = true;
            o = NULL;
        }
    }
```

Code after:
    /* Don't abort when the key cannot be found. Non-existing keys are empty
     * hashes, where HGETDEL should respond with a series of null bulks. */
    robj *o = lookupKeyWrite(c->db, c->argv[1]);
    if (checkType(c, o, OBJ_HASH)) return;

    bool hash_volatile_items = hashTypeHasVolatileFields(o);

    /* Reply with array of values and delete at the same time */
    addReplyArrayLen(c, num_fields);
    for (i = fields_index; i < c->argc; i++) {
        addHashFieldToReply(c, o, c->argv[i]->ptr);

        /* If hash doesn't exist, continue as already replied with NULL */
        if (o == NULL) continue;
        if (hashTypeDelete(o, c->argv[i]->ptr)) {
            deleted++;
            if (hashTypeLength(o) == 0) {
                if (hash_volatile_items) dbUntrackKeyWithVolatileItems(c->db, o);
                dbDelete(c->db, c->argv[1]);
                keyremoved = true;
                o = NULL;
            }
        }
    }

    if (deleted) {
        if (!keyremoved && hash_volatile_items != hashTypeHasVolatileFields(o)) {
            dbUpdateObjectWithVolatileItemsTracking(c->db, o);
        }
        signalModifiedKey(c, c->db, c->argv[1]);
        notifyKeyspaceEvent(NOTIFY_HASH, "hdel", c->argv[1], c->db->id);
        if (keyremoved) notifyKeyspaceEvent(NOTIFY_GENERIC, "del", c->argv[1], c->db->id);
        server.dirty += deleted;
    }
}

void hlenCommand(client *c) {
    robj *o;

    if ((o = lookupKeyReadOrReply(c, c->argv[1], shared.czero)) == NULL || checkType(c, o, OBJ_HASH)) return;

    addReplyLongLong(c, hashTypeLength(o));
}

void hstrlenCommand(client *c) {
    robj *o;

    if ((o = lookupKeyReadOrReply(c, c->argv[1], shared.czero)) == NULL || checkType(c, o, OBJ_HASH)) return;
    addReplyLongLong(c, hashTypeGetValueLength(o, c->argv[2]->ptr));
}