Make last_log_id an Option<LogId>, or it can not express an uninitialized state

[drmingdrmer]

openraft/openraft/src/core/mod.rs

Line 140 in 344d444

last_log_id: LogId,

[github-actions]

👋 Thanks for opening this issue!

Get help or engage by:

• /help : to print help messages.
• /assignme : to assign this issue to you.

[Veeupup]

/assignme

[YangKian]

Hi~ I have an idea to discuss with you guys, make last_log_id an Option<LogId> may introduce a lot of unwrap operations, fortunately, PR #94 adds a return error type for many APIs, so I would like to add a helper function for RaftCore:

impl RaftCore {
    fn get_last_log_id(&self) -> Result<LogId, SomeErr>
}

use option.ok_or_else() to convert None to an Err.

What do you think? Also regarding the error type returned, should I add a new error type or return an existing error? Would like to hear your suggestions! @drmingdrmer @Veeupup

[drmingdrmer]

IMHO, the root cause of there being a lot of unwrap() is other fields that have not yet been updated, such as matched and last_applied, which will be replaced with Option in future.

Returning an error is no better than an unwrap() because at the top most level it has nothing to do but just panic.

My suggestion is:

• For now, just use unwrap() if necessary, to keep simplicity.
• Find some ways to avoid using unwrap(). E.g., when comparing a a: LogId and b: Option<LogId> it would be more elegant by doing it with Some(a) >= b, instead of a >= b.unwrap().

[YangKian]

I see, thanks for the advice!