Skip to content

Matin_Frontend (Update 2)#55

Open
MatinMirzaei wants to merge 5 commits intodevelopfrom
matin_frontend
Open

Matin_Frontend (Update 2)#55
MatinMirzaei wants to merge 5 commits intodevelopfrom
matin_frontend

Conversation

@MatinMirzaei
Copy link
Contributor

@MatinMirzaei MatinMirzaei commented May 24, 2022

My Account page (updated)
Post a Book page (updated)
Find a Book page (updated)
Dummy data (created)
Book Detail View (created)

Comment on lines 12 to 20
struct Books: Identifiable {
let id = UUID()
let imageName: String
let title: String
let author: String
let edition: String
let isbn: String
let uploadDate: String
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change needed to the data structure itself, but you may want to coordinate on your Book data modeling between you, Robert and Justin to sync them up eventually. For example, here's @RJB888 's latest

class Book: Codable, Identifiable { let id = UUID() let title: String let author: String let isbn: String let price: Decimal }


struct BookDetailView: View {

var books: Books
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable would make more sense using the singular book rather than the books

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that makes sense. I will change that to "book".

import Foundation
import SwiftUI

struct Books: Identifiable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This struct represents a single book and its name would make more sense using the singular Book rather than the plural Books

Copy link
Contributor

@Bansenauer-Cascadia Bansenauer-Cascadia May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the logo on most screens is set to Navigate to an undefined location
AND, the Help screen does not seem to be accessible from any other screen.

Copy link
Contributor Author

@MatinMirzaei MatinMirzaei May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Xcode did not like it when I changed "Books" to "Book".

Copy link
Contributor

@RJB888 RJB888 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs changes. See comments.

import Foundation
import SwiftUI

struct Books: Identifiable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is "Books" ? is it an array of books or a single book object? Why is it plural. Why do you need to identify an object as "Books" when we already have a Book object?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is way more data on a book than we are ready for. We are doing just an MVP here, we aren't set up for full functionality.

}

struct BookList {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A hard-coded list of items should be somewhere else that can be programmatically accessed. This allows us to use it or something else interchangeably and does not bloat the actual code with dummy data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants