Aggregated account data #196
Conversation
pifragile
commented
Apr 5, 2022
pifragile
commented
- Closes Custom RPC: Aggregated Account Data #157
| let mut meetup_location_index: Option<MeetupIndexType> = None; | ||
| let mut meetup_time: Option<T::Moment> = None; | ||
| let mut meetup_registry: Option<Vec<T::AccountId>> = None; | ||
| let mut meetup_index: Option<MeetupIndexType> = None; | ||
|
|
There was a problem hiding this comment.
Why do you predefine those fields, we same some lines of code if this is defined in place.
There was a problem hiding this comment.
i do it because i use a bunch of if statements to assign those variables. if i dont predefine them here, they woule be undefined in some cases. if there is a more elegant solution to this, please give me a hint :)
There was a problem hiding this comment.
It needs a bit of thinking, but I think you can. The code is hard to read like this, I'd want to change that. Basically, all the fields are null if participant_meetup_index is None correct? Then I'd propose the following:
let participant_meetup_index = match Self::get_meetup_index() {
Some(p_index) => p_index,
None => return Some(AggregatedAccountDataPersonal::new_unassigned(participant_type));
}
.... continue as usual
This pattern would, however, require extracting get_agregated_account_data_personal as advised above. I think you did not see that message because it was a hidden conversation.
|
@brenzi I only have code-quality remarks. I think from the functionality it looks fine, so from my point of view we can merge this to get it into the parachain. |
|
well, an unwarp() in the runtime is not only a code quality problem. It is serious. but it's not part of an extrinsic execution, only rpc, so not strictly critical |
|
Thanks for your very modular fixes. That makes it a bliss to review! |
