C++ coding style concerns

[ni4]

Description

As part of C++ refactoring I've got to the point where started moving signature_get_something/signature_set_somethhing/signature_do_something functions to the pgp_signature_t methods. And since we do not have reference example in the code, I'd like to discuss do/not to do and other coding style concerns so we'd be stick to the single standard in future. Following is the pgp_signature_t declaration I have at the moment (with unrelated parts cut out), with questions and concerns below.

typedef struct pgp_signature_t {
    pgp_sig_type_t   type_;

    pgp_signature_t(const pgp_signature_t &src);
    pgp_signature_t(pgp_signature_t &&src);
    pgp_signature_t &operator=(pgp_signature_t &&src);
    pgp_signature_t &operator=(const pgp_signature_t &src);
    bool             operator==(const pgp_signature_t &src) const;
    bool             operator!=(const pgp_signature_t &src) const;
    ~pgp_signature_t();

    /* @brief Get signature's type */
    pgp_sig_type_t
    type() const
    {
        return type_;
    };
    void
    set_type(pgp_sig_type_t atype)
    {
        type_ = atype;
    };

    /* @brief Get v4 signature's subpacket of the specified type and hashedness */
    pgp_sig_subpkt_t *      get_subpkt(pgp_sig_subpacket_type_t stype, bool hashed = true);
    const pgp_sig_subpkt_t *get_subpkt(pgp_sig_subpacket_type_t stype,
                                       bool                     hashed = true) const;
    /* @brief Check whether v4 signature has subpacket of the specified type/hashedness */
    bool has_subpkt(pgp_sig_subpacket_type_t stype, bool hashed = true) const;
    /* @brief Check whether signature has signing key id (via v3 field, or v4 key id/key fp
     * subpacket) */
    bool has_keyid() const;
    /** 
     * @brief Get signature's signing key id
     * @param id reference to return key identifier
     * @return true on success (key id is available) or false otherwise
     */
    bool get_keyid(pgp_key_id_t &id) const;

    /**
     * @brief Add subpacket of the specified type to v4 signature
     * @param type type of the subpacket
     * @param datalen length of the subpacket body
     * @param reuse replace already existing subpacket of the specified type if any
     * @return reference to the subpacket structure or throws an exception
     */
    pgp_sig_subpkt_t &add_subpkt(pgp_sig_subpacket_type_t type, size_t datalen, bool reuse);

    /**
     * @brief Remove signature's subpacket
     * @param subpkt subpacket to remove. If not in the subpackets list then no action is
     * taken.
     */
    void remove_subpkt(pgp_sig_subpkt_t *subpkt);

    /**
     * @brief Check whether signature packet matches one-pass signature packet.
     * @param onepass reference to the read one-pass signature packet
     * @return true if sig corresponds to onepass or false otherwise
     */
    bool matches_onepass(const pgp_one_pass_sig_t &onepass) const;
} pgp_signature_t;

1. Field and getter/setter naming: type_, type(), set_type(). Is these good to go or better stick to other strategy?
2. Doxygen comments in class declaration. Doesn't it make class to look cluttered?
3. bool get_keyid(pgp_key_id_t &id) const;. What is better - this one, or throwing const pgp_key_id_t &keyid() const? Since keyid value is not always available.

[ni4]

CC @rnpgp/developers

[CAMOBAP]

1. Looks good to me
2. While you keep the same code formatting it looks good, we have .clang-format this is awesome
3. Can we have both, maybe some developers who will use our library disabling exceptions for some reason

[ronaldtse]

LGTM, how do we move forward with this? ping @dewyatt

[ni4]

@ronaldtse There is a WIP PR here: https://github.com/rnpgp/rnp/pull/1325 Will get back to it today or tomorrow.

[ni4]

Closing this as a lot of C++ code was already written and merged.