Dashboard Card created for personal project

[clc80]

Description

Fixes # (issue # from issue tracker)

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Change Status

• Complete, tested, ready to review and merge
• Complete, but not tested (may need new tests)
• Incomplete/work-in-progress, PR is for discussion/feedback

How Has This Been Tested?

• Unit Tests
• Integration Tests
• User Tests

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• My code has been reviewed by at least one peer
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• There are no merge conflicts

[jaredsinclair]

All this setup should be in DashboardCard, part of its private implementation details. The images and labels should be properties of the card.

The DashboardCard should expose a public API for providing images and text that can be applied to its subviews. There are two ways of going about it:

1. The properties for images and labels are private, and DashboardCard provides a method likefunc configure(with model: DashboardCard.Model), which would take a Model struct with UIImage and String properties that DashbaordCard can decorate itself with.

2. The properties for images and labels are not private, and the object using the card (in this case, the view controller) sets their properties: card.someImageView.image = blah, card.someLabel.text = blah, etc.

There are arguments for and against both options. What do you think? Which one would you argue for, and why?

[clc80]

I went with the first option because as a developer I find it easier to fill in the preset items that are required when initializing the component. So in my DashboardCard file should I make the properties private?

I was trying to figure out how to change the background color of the Card itself I tried it in both the ViewController and the DashboardCard and neither is making the background color a different color. What am I missing? I'm sure it's something simple that I just can't see.

[jaredsinclair]

Hi. I need to clarify what I meant by a "configure" method. What you have in your updated code isn't what I was recommended. When you're writing a custom view, there are — very, very roughly speaking — two kinds of configuration:

1. Initial setup done once. - This is stuff like adding subviews, establishing AutoLayout constraints, choosing fonts and colors, etc. These are things that a view would set up all by itself, either during init or during awakeFromNib, etc. What's important to note here is that this is setup that is common to any content the view might display. For example, if you were writing a Twitter client, the layout code in a tweet cell that puts the avatar over here and the user name over there is something you would set once, even though that tweet cell might get re-used many times to display many different tweets.

2. Updating the view to display new content. - This is stuff that happens once, twice, or a hundred times during the lifetime of the view, such as: setting the image property on a UIImageView, setting the text of a label, etc. In my example of the tweet cell, this is setting the image property on the avatar image view, or the text property of a user name label. This happens frequently.

So with your Dashboard card, you also have two kinds of setup. Setup Number 1 is what you currently have implemented in private func configure(). There's also additional setup needed to apply a particular image or text to the DashboardCard's subviews. Currently you are doing this in init, but that's discouraged. It's better to design the API of your view to be initialized once and re-used as needed.

Here's a stubbed-in version explaining what I mean:

class DashboardCard: UIView {

    struct Model {
        let image1: UIImage
        let image2: UIImage
        let title: String
        let subtitle: String
    }

    private let imageView1 = UIImageView()
    private let imageView2 = UIImageView()
    private let titleLabel = UILabel()
    private let subtitleLabel = UILabel()

    override init(frame: CGRect) {
        super.init(frame: frame)
        initialSetup()
    }

    func configure(with model: Model) {
        imageView1.image = model.image1
        imageView2.image = model.image2
        titleLabel.text = model.title
        subtitleLabel.text = model.subtitle
    }

    private func initialSetup() {
        // add subviews
        // set up layout constraints
        // set background colors, etc.
    }

}

What this setup allows you to do is make all the details that only DashboardCard should need to know about as private matters. It lets another object that needs to use a DashboardCard just do the following:

class SomeViewControllerSomewhere: UIViewController {

    @IBOutlet private var dashboardCard: DashboardCard!

    func didRefreshAndGetSomeDataOrWhatever(data: SomeModelType) {
        let cardModel = DashboardCard.Model(image1: data.i1, image2: data.i2, title: data.t, subtitle: data.s)
        dashboardCard.configure(with: cardModel)
    }

}

What you can see here is that SomeViewControllerSomewhere doesn't need to know anything about the private details of DashboardCard or it's subviews. All it needs, other than adding a DashboardCard to it's view hierarchy, is to call configure(with: DashboardCard.Model) on the card, passing the model values it wants the card to present. Don't pass actual views (don't pass image views or labels). Instead, pass values: images, strings, bools, that sort of thing.

[jaredsinclair]

I recommend making these private unless there's a good reason for another object to be able to reach in and access these things directly.

Suggested change

	var image1 = UIImageView()
	var image2 = UIImageView()
	let title = UILabel()
	let subTitle = UILabel()
	private var image1 = UIImageView()
	private var image2 = UIImageView()
	private let title = UILabel()
	private let subTitle = UILabel()

[jaredsinclair]

Watch out for little inconsistencies of spacing like this. It can feel frustrating that this matters, but I assure you there are plenty of technical reviewers out there who will notice this kind of thing.

Suggested change

	super .init(frame: frame)
	super.init(frame: frame)

[jaredsinclair]

See my long-winded comment above. This shouldn't take any arguments, just override init(frame:)

Suggested change

	init(image1: UIImageView, image2: UIImageView, title: String, subTitle: String) {
	super .init(frame: .zero)
	self.image1 = image1
	self.image2 = image2
	self.title.text = title
	self .subTitle.text = subTitle
	configure()
	}
	override init(frame: CGRect) {
	super.init(frame: frame)
	configure()
	}

[jaredsinclair]

This method has a mix of including self. and excluding it. Make sure you pick one style and stick with it, at least within a file or a feature. My recommendation is to exclude self. when it can be, that's what most places are doing now.

Suggested change

	layer.cornerRadius = 10
	self.backgroundColor = .systemPink
	self.addSubview(image1)
	self.addSubview(image2)
	self.addSubview(title)
	self.addSubview(subTitle)
	layer.cornerRadius = 10
	backgroundColor = .systemPink
	addSubview(image1)
	addSubview(image2)
	addSubview(title)
	addSubview(subTitle)

[jaredsinclair]

Great catch. It is really easy to forget to do this with views that are initialized programmatically. I hate that's what UIKit still does.

[jaredsinclair]

Nice, using NSLayoutConstraint.activate and the anchor APIs. I like to use these, too, I think they're a good blend of ease-of-use and familiarity.

[jaredsinclair]

The reason you're not seeing a background color is because the DashboardCard has an ambiguous height, given the constraints you have here. I opened your app in Xcode's visual debugger to help diagnose this:

UIKit can't determine what the height is supposed to be, so it's using 0, which is the height it's initialized with. The reason the height is ambiguous is because your constraints don't have a contiguous chain of constraints from the top of the card to the bottom of the card. Right now the chain is broken:

• START: top of the card
• top of the card to top of image 1
• bottom of image1 to top of title label
• bottom of title label to top of subtitle label
• BROKEN
• END: bottom of the card

You need a constraint between the bottom of the subtitle label and the bottom of the card.