Skip to content

Commit

Permalink
chore(device): Improve error handling by using Result instead of panic
Browse files Browse the repository at this point in the history
  • Loading branch information
Holzhaus committed Oct 13, 2022
1 parent 6ae962a commit 8a44d90
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 17 deletions.
34 changes: 20 additions & 14 deletions src/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,28 +128,26 @@ impl DeviceExport {
pub fn get_playlists(&self) -> crate::Result<Vec<PlaylistNode>> {
match &self.pdb {
Some(pdb) => pdb.get_playlists(),
None => Ok(Vec::new()),
None => Err(crate::Error::NotLoadedError),
}
}

/// Get the entries for a single playlist.
#[must_use]
pub fn get_playlist_entries(
&self,
id: PlaylistTreeNodeId,
) -> Box<dyn Iterator<Item = crate::Result<(u32, TrackId)>> + '_> {
) -> crate::Result<impl Iterator<Item = crate::Result<(u32, TrackId)>> + '_> {
match &self.pdb {
Some(pdb) => Box::new(pdb.get_playlist_entries(id)),
None => Box::new(std::iter::empty()),
Some(pdb) => Ok(pdb.get_playlist_entries(id)),
None => Err(crate::Error::NotLoadedError),
}
}

/// Get the tracks.
#[must_use]
pub fn get_tracks(&self) -> Box<dyn Iterator<Item = crate::Result<Track>> + '_> {
pub fn get_tracks(&self) -> crate::Result<impl Iterator<Item = crate::Result<Track>> + '_> {
match &self.pdb {
Some(pdb) => Box::new(pdb.get_tracks()),
None => Box::new(std::iter::empty()),
Some(pdb) => Ok(pdb.get_tracks()),
None => Err(crate::Error::NotLoadedError),
}
}
}
Expand Down Expand Up @@ -393,12 +391,16 @@ impl Pdb {
self.get_rows_by_page_type(PageType::PlaylistTree)
.map(|row| {
if let Row::PlaylistTreeNode(playlist_tree) = row {
playlist_tree
Ok(playlist_tree)
} else {
unreachable!("encountered non-playlist tree row in playlist table");
Err(crate::Error::IntegrityError(
"encountered non-playlist tree row in playlist table",
))
}
})
.for_each(|row| playlists.entry(row.parent_id).or_default().push(row));
.try_for_each(|row| {
row.map(|node| playlists.entry(node.parent_id).or_default().push(node))
})?;

fn get_child_nodes<'a>(
playlists: &'a HashMap<PlaylistTreeNodeId, Vec<&PlaylistTreeNode>>,
Expand Down Expand Up @@ -446,7 +448,9 @@ impl Pdb {
None
}
} else {
unreachable!("encountered non-playlist tree row in playlist table");
Some(Err(crate::Error::IntegrityError(
"encountered non-playlist tree row in playlist table",
)))
}
})
}
Expand All @@ -457,7 +461,9 @@ impl Pdb {
if let Row::Track(track) = row {
Ok(track.clone())
} else {
unreachable!("encountered non-track row in track table");
Err(crate::Error::IntegrityError(
"encountered non-track row in track table",
))
}
})
}
Expand Down
9 changes: 6 additions & 3 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ fn list_playlists(path: &Path) -> rekordcrate::Result<()> {
.for_each(|child| walk_tree(export, child, level + 1));
}
PlaylistNode::Playlist(playlist) => {
let num_tracks = export.get_playlist_entries(playlist.id).count();
let num_tracks = export
.get_playlist_entries(playlist.id)
.expect("failed to get playlist entries")
.count();
println!("{}🗎 {} ({} tracks)", indent, playlist.name, num_tracks)
}
};
Expand All @@ -106,7 +109,7 @@ fn export_playlists(path: &Path, output_dir: &PathBuf) -> rekordcrate::Result<()
export.load_pdb()?;
let playlists = export.get_playlists()?;
let mut tracks: HashMap<TrackId, Track> = HashMap::new();
export.get_tracks().try_for_each(|result| {
export.get_tracks()?.try_for_each(|result| {
if let Ok(track) = result {
tracks.insert(track.id, track);
Ok(())
Expand All @@ -129,7 +132,7 @@ fn export_playlists(path: &Path, output_dir: &PathBuf) -> rekordcrate::Result<()
}
PlaylistNode::Playlist(playlist) => {
let mut playlist_entries = export
.get_playlist_entries(playlist.id)
.get_playlist_entries(playlist.id)?
.collect::<rekordcrate::Result<Vec<(u32, TrackId)>>>()?;
playlist_entries.sort_by_key(|entry| entry.0);

Expand Down
8 changes: 8 additions & 0 deletions src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,17 @@ pub enum RekordcrateError {
#[error(transparent)]
ParseError(#[from] binrw::Error),

/// Represents a failure to validate a constraint.
#[error("failed integrity constraint: {0}")]
IntegrityError(&'static str),

/// Represents an `std::io::Error`.
#[error(transparent)]
IOError(#[from] std::io::Error),

/// Represents an `std::io::Error`.
#[error("component not loaded")]
NotLoadedError,
}

/// Type alias for results where the error is a `RekordcrateError`.
Expand Down

0 comments on commit 8a44d90

Please sign in to comment.