3
\$\begingroup\$

I am adding some constraints for my auto layout.

My method calling the methods creating the constraints:

// all
if displayUnitConversion && displayCode {
    if currency.isDefaultCurrency {
        buildConstraintsForDefaultCurrency(view1: computedRateLabel, sizeView1: 37, view2: codeLabel, sizeView2: 27)
    } else {
        switch (rowDisplayType) {
        case 0:
            buildConstraintsFor(view1: computedRateLabel, sizeView1: 30, view2: codeLabel, sizeView2: 18, view3: rateLabel, sizeView3: 15)
        case 1:
            buildConstraintsFor(view1: rateLabel, sizeView1: 18, view2: computedRateLabel, sizeView2: 30, view3: codeLabel, sizeView3: 15)
        case 2:
            buildConstraintsFor(view1: computedRateLabel, sizeView1: 30, view2: rateLabel, sizeView2: 18, view3: codeLabel, sizeView3: 15)
        default:
            break
        }
    }
    codeLabel.hidden = false
    rateLabel.hidden = false

    computedRateLabel.font = computedRateLabel.font.fontWithSize(CGFloat(25))
    codeLabel.font = codeLabel.font.fontWithSize(CGFloat(16))
    rateLabel.font = rateLabel.font.fontWithSize(CGFloat(12))
} // rate only
else if !displayUnitConversion && !displayCode {
    buildConstraintsFor(view1: computedRateLabel, sizeView1: 45)
    codeLabel.hidden = true
    rateLabel.hidden = true

    computedRateLabel.font = computedRateLabel.font.fontWithSize(CGFloat(30))
} // rate and code
else if !displayUnitConversion && displayCode {
    buildConstraintsFor(view1: computedRateLabel, sizeView1: 35, view2: codeLabel, sizeView2: 25)
    codeLabel.hidden = false
    rateLabel.hidden = true
    computedRateLabel.font = computedRateLabel.font.fontWithSize(CGFloat(30))
    codeLabel.font = codeLabel.font.fontWithSize(CGFloat(15))
} // rate and unit conversion
else if displayUnitConversion && !displayCode {
    if currency.isDefaultCurrency {
        buildConstraintsForDefaultCurrency(view1: computedRateLabel, sizeView1: 61)
    } else {
        switch (rowDisplayType) {
        case 0, 2:
            buildConstraintsFor(view1: computedRateLabel, sizeView1: 35, view2: rateLabel, sizeView2: 25)
        case 1:
            buildConstraintsFor(view1: rateLabel, sizeView1: 25, view2: computedRateLabel, sizeView2: 35)
        default:
            break
        }
    }
    codeLabel.hidden = true
    rateLabel.hidden = false
    computedRateLabel.font = computedRateLabel.font.fontWithSize(CGFloat(30))
    rateLabel.font = rateLabel.font.fontWithSize(CGFloat(16))
}

Methods creating constraints:

// Default Currency
// build constraints for rate and unit conversion or for rate and currency code
func buildConstraintsForDefaultCurrency(#view1: UILabel, sizeView1: CGFloat) {
    let viewsDictionary = ["flagImageView": flagImageView, "view1": view1, "historyButton": historyButton]
    addCommonConstraints()
    view.addConstraints(NSLayoutConstraint.constraintsWithVisualFormat("V:|-1-[view1(==\(sizeView1))]-1-|", options: .AlignAllLeading | .AlignAllTrailing, metrics: nil, views: viewsDictionary))
    view.addConstraints(NSLayoutConstraint.constraintsWithVisualFormat("H:[flagImageView]-[view1]-[historyButton]", options: .AlignAllBottom, metrics: nil, views: viewsDictionary))
}

// build constraints for rate and unit conversion or for rate and currency code
func buildConstraintsForDefaultCurrency(#view1: UILabel, sizeView1: CGFloat, view2: UILabel, sizeView2: CGFloat) {
    let viewsDictionary = ["flagImageView": flagImageView, "view1": view1, "view2": view2, "historyButton": historyButton]
    addCommonConstraints()
    view.addConstraints(NSLayoutConstraint.constraintsWithVisualFormat("V:|-1-[view1(==\(sizeView1))]-1-[view2(==\(sizeView2))]-1-|", options: .AlignAllLeading | .AlignAllTrailing, metrics: nil, views: viewsDictionary))
    view.addConstraints(NSLayoutConstraint.constraintsWithVisualFormat("H:[flagImageView]-[view2]-[historyButton]", options: .AlignAllBottom, metrics: nil, views: viewsDictionary))
}


// build constraints for rate only
func buildConstraintsFor(#view1: UIView, sizeView1: CGFloat) {
    let viewsDictionary = ["flagImageView": flagImageView, "view1": view1, "historyButton": historyButton]
    addCommonConstraints()
    view.addConstraints(NSLayoutConstraint.constraintsWithVisualFormat("V:|-1-[view1(==\(sizeView1))]-1-|", options: nil, metrics: nil, views: viewsDictionary))
    view.addConstraints(NSLayoutConstraint.constraintsWithVisualFormat("H:[flagImageView]-[view1]-[historyButton]", options: .AlignAllBottom, metrics: nil, views: viewsDictionary))
}

// build constraints for rate and unit conversion or for rate and currency code
func buildConstraintsFor(#view1: UILabel, sizeView1: CGFloat, view2: UILabel, sizeView2: CGFloat) {
    let viewsDictionary = ["flagImageView": flagImageView, "view1": view1, "view2": view2, "historyButton": historyButton]
    addCommonConstraints()
    view.addConstraints(NSLayoutConstraint.constraintsWithVisualFormat("V:|-1-[view1(==\(sizeView1))]-1-[view2(==\(sizeView2))]-1-|", options: .AlignAllLeading | .AlignAllTrailing, metrics: nil, views: viewsDictionary))
    view.addConstraints(NSLayoutConstraint.constraintsWithVisualFormat("H:[flagImageView]-[view2]-[historyButton]", options: .AlignAllBottom, metrics: nil, views: viewsDictionary))
}

// build constraints for rate, unit conversion and currency code
func buildConstraintsFor(#view1: UIView, sizeView1: CGFloat, view2: UIView, sizeView2: CGFloat, view3: UIView, sizeView3: CGFloat) {
    let viewsDictionary = ["flagImageView": flagImageView, "view1": view1, "view2": view2, "view3": view3, "historyButton": historyButton]
    addCommonConstraints()
    view.addConstraints(NSLayoutConstraint.constraintsWithVisualFormat("V:|-1-[view1(==\(sizeView1))]-1-[view2(==\(sizeView2))]-1-[view3(==\(sizeView3))]-1-|", options: .AlignAllLeading | .AlignAllTrailing, metrics: nil, views: viewsDictionary))
    view.addConstraints(NSLayoutConstraint.constraintsWithVisualFormat("H:[flagImageView]-[view3]-[historyButton]", options: .AlignAllBottom, metrics: nil, views: viewsDictionary))
}

I there a better way to do this than creating 5 methods that are pretty similar?

\$\endgroup\$
  • \$\begingroup\$ Use interface builder. \$\endgroup\$ – nhgrif Jul 30 '15 at 12:12
  • \$\begingroup\$ I can't, they might change during run time \$\endgroup\$ – Nico Jul 30 '15 at 12:13
  • \$\begingroup\$ Use interface builder and hook up outlets to the constraints so you can change them during run time. \$\endgroup\$ – nhgrif Jul 30 '15 at 12:13
  • \$\begingroup\$ those constraints are for the same objects. How can I do it in IB? By hooking them all up and removing them at runtime and adding them later when needed thanks to the outlets? Because I think this way is bit messy and not very clear if I want to add some modifications later \$\endgroup\$ – Nico Jul 30 '15 at 12:18
3
\$\begingroup\$

Use interface builder.

Use interface builder to set up your constraints. If they need to be altered at run time, connect IBOutlets for the constraints and alter them at run time.

You can look at this Stack Overflow answer for a simplified approach at changing constraints at run time based on something you set up in interface builder.

What's not immediately obvious but that you should certainly keep in mind is that this approach saves us some run-time CPU time because we don't have to programmatically create the constraints every single time. The constraints are encoded into the interface builder file and loaded into memory when the view is loaded. It still takes some processor time to swap out the constraints, but at the end of the day this is much more efficient (and significantly cleaner) than what you have.


No more magic numbers.

You've got magic numbers all over the place, and several of them are repeated. If you ever change your mind on what value to use, now you have to change it in several places and hope you don't make a typo.

You've got a lot here, so I'm not going to even attempt coming up with a constant name for all of them, but suffice to say, all of the stuff that shows up in red on this page (all of the literal numbers) should instead be a named constant.

This includes the values for the case statements in the switch. These should be an enum.


About these build methods...

// build constraints for rate and unit conversion or for rate and currency code
func buildConstraintsForDefaultCurrency(#view1: UILabel, sizeView1: CGFloat)

All of your "build" methods all have the same set of problems.

  1. They're called "build", but they actually "add" constraints. These method names should more clearly indicate that they're actually adding constraints. Otherwise, if they're called build, they should return constraints.
  2. They manage to all require a comment describing what they do... and the comment isn't even in the useful Appledoc style so that Xcode will help you out with it later on. These all need better names and Appledoc style comments.
  3. When you're numbering variables, you need an array. So, why not let this method take a variable number of arguments?

    func setupConstraints(views forViews:[UIView], sizes withSizes:[CGFloat])
    

    This will require a bit of extra logic to validate inputs and to figure out exactly how to set up the constraints, but once that's done, it's much easier to call and use this method now.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Thanks for your feedback. I made some changes based on your remarks. \$\endgroup\$ – Nico Aug 4 '15 at 10:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.