Swift UITableViewCell class to show cameras, ratings for a car wash and other information

Question

I have an UITableViewCell class that written in Swift. And it seems to me that I did it the wrong way. It works fine, but I want improve the code or refactor it somehow to make it look better.

Any ideas will be helpful!

Here is the code:

class cwCell: UITableViewCell {

//MARK: - Outlets
@IBOutlet weak var nameLbl                  : UILabel!
@IBOutlet weak var ratingImg                : UIImageView!
@IBOutlet weak var ratingLbl                : UILabel!
@IBOutlet weak var bottomInformationLbl     : UILabel!
@IBOutlet weak var cameraView               : UIView!
@IBOutlet weak var favImg                   : UIImageView!
@IBOutlet weak var activityIndicator        : UIActivityIndicatorView!
@IBOutlet weak var gettingImgLbl            : UILabel!
@IBOutlet weak var hlsFrameView             : VideoContainerView!
@IBOutlet weak var sleepImg                 : UIImageView!
@IBOutlet weak var cameraStatusImg          : UIImageView!
@IBOutlet weak var blindCWLbl               : UILabel!
@IBOutlet weak var voteStackView            : UIStackView!
@IBOutlet weak var voteNumberLbl            : UILabel!
@IBOutlet weak var voteView                 : UIView!
@IBOutlet weak var voteButton               : UIButton!

//MARK: - Variables
var cw                                      : CarWash!
var request                                 : Request?
var mainListVC                              : MainListVC!
var player                                  : AVPlayer?
var playerLayer                             : AVPlayerLayer?
let audioSession                            = AVAudioSession.sharedInstance()
var camArray                                : [JSON]? = { return VeedoServices.shared.veedoCamerasArray }()

//MARK: - Override UIView Methods
override func awakeFromNib() {
    super.awakeFromNib()

    // Setup shadow and cornder radius for cell
    self.cameraView.layer.cornerRadius = 8
    self.cameraView.layer.masksToBounds = true

    blindCWLbl.isHidden = true
    gettingImgLbl.isHidden = false
    ratingImg.layer.masksToBounds = false
    ratingImg.isHidden = true
    ratingImg.clipsToBounds = false
    ratingLbl.isHidden = true
    sleepImg.isHidden = true
    favImg.clipsToBounds = true
    cameraStatusImg.clipsToBounds = true
    voteStackView.isHidden = true
    voteView.viewWithTwoRoundedCorners()
    voteButton.buttonWithTwoRoundedCorners()
}

override func prepareForReuse() {
    hlsFrameView.layer.sublayers = nil
    DispatchQueue.main.async { self.killPlayer() }
}

//MARK: - Configuring cell
func configCell(_ cw: CarWash, sortFilterChosen: CarwashFilters? = .nearest, streamUrl: String?) {

    self.cw = cw

    nameLbl.text = cw.cwTitle

    if cw.cwFavorite == 0 {
        favImg.isHidden = true

    } else if cw.cwFavorite == 1 {
        favImg.isHidden = false
        favImg.image = UIImage(named: "icBookmarkAdded.png")
    }

    if camArray != nil {
        for item in camArray! {

            if cw.cwCameraID == item["stream_id"].string {

                blindCWLbl.isHidden = true
                voteStackView.isHidden = true
                hlsFrameView.isHidden = false
                gettingImgLbl.isHidden = false

                if item["onair"].int == 1 {
                    cameraStatusImg.image = UIImage(named: "icCameraOn.png")
                    gettingImgLbl.text = "Получаю изображение с камеры..."
                    sleepImg.isHidden = true
                    activityIndicator.startAnimating()

                } else if item["onair"].int == 0 {
                    cameraStatusImg.image = UIImage(named: "icCameraOff.png")
                    gettingImgLbl.text = "Ведутся техничекие работы..."
                    activityIndicator.stopAnimating()
                }

            } else if cw.cwCameraID.isEmpty {
                cameraStatusImg.image = UIImage(named: "icCameraOff.png")
                blindCWLbl.isHidden = false
                hlsFrameView.isHidden = true
                gettingImgLbl.isHidden = true
                sleepImg.isHidden = true
                voteStackView.isHidden = false
                activityIndicator.stopAnimating()
                getVotes(for: cw.cwID)

                if UserDefaults.standard.value(forKey: cw.cwID) != nil {
                    disableVoteButton()
                } else {
                    enableVoteButton()
                }

            }
        }

    } else {
        cameraStatusImg.image = UIImage(named: "icCameraOff.png")
        gettingImgLbl.text = "Ведутся техничекие работы..."
        sleepImg.isHidden = true
        activityIndicator.stopAnimating()
    }

    if !cw.isCarwashOpen() {
        gettingImgLbl.text =
        """
        В настоящее время мойка закрыта!
        Работаем с \(cw.timeToOpen) до \(cw.timeToClose) часов!
        """
        sleepImg.isHidden = false
        activityIndicator.stopAnimating()
        DispatchQueue.main.async{ self.killPlayer() }
    }

    if streamUrl != nil {
        gettingImgLbl.text = "Получаю изображение с камеры..."
        sleepImg.isHidden = true
        activityIndicator.startAnimating()
        DispatchQueue.main.async{ self.runPlayer(with: streamUrl!) }
    }

    /// setting UI conditions depending on current carwash's sort filter
    if sortFilterChosen != nil {
        switch sortFilterChosen! {

        case .nameDown:
            bottomInformationLbl.isHidden = false
            bottomInformationLbl.text = cw.cwAddress
            ratingImg.isHidden = true
            ratingLbl.isHidden = true

        case .nameUp:
            bottomInformationLbl.isHidden = false
            bottomInformationLbl.text = cw.cwAddress
            ratingImg.isHidden = true
            ratingLbl.isHidden = true

        case .nearest:
            bottomInformationLbl.isHidden = false

            if cw.nearestCW() == 0.0 {
                bottomInformationLbl.text = "Вы на месте!"

            } else {
                bottomInformationLbl.text = String(format: "Расстояние до мойки %.1f км", cw.nearestCW() / 1000)
            }

            ratingImg.isHidden = true
            ratingLbl.isHidden = true

        case .ratingDown:
            bottomInformationLbl.isHidden = true
            ratingImg.isHidden = false
            ratingLbl.isHidden = false

        case .ratingUp:
            bottomInformationLbl.isHidden = true
            ratingImg.isHidden = false
            ratingLbl.isHidden = false

        case .favorites:
            bottomInformationLbl.isHidden = false
            bottomInformationLbl.text = String(format: "Расстояние до мойки %.1f км", cw.nearestCW() / 1000)
            ratingImg.isHidden = true
            ratingLbl.isHidden = true

        default:
            break
        }

    } else {
        bottomInformationLbl.isHidden = false
        bottomInformationLbl.text = cw.cwAddress
        ratingImg.isHidden = true
        ratingLbl.isHidden = true
    }

    /// Setting rating images
    if cw.cwRating != "" {
        let rating = HelpServices.shared.approximateValue(cw.cwRating)

        switch rating {

        case 0.0..<0.1: ratingLbl.text = "0.5"
        case 0.1..<0.2: ratingLbl.text = "1.0"
        case 0.2..<0.3: ratingLbl.text = "1.5"
        case 0.3..<0.4: ratingLbl.text = "2.0"
        case 0.4..<0.5: ratingLbl.text = "2.5"
        case 0.5..<0.6: ratingLbl.text = "3.0"
        case 0.6..<0.7: ratingLbl.text = "3.5"
        case 0.7..<0.8: ratingLbl.text = "4.0"
        case 0.8..<0.9: ratingLbl.text = "4.5"
        case 0.9..<1.0: ratingLbl.text = "5.0"

        default:
            break
        }
    }
}

func runPlayer(with urlString: String) {
    player = AVPlayer(url: URL(string: urlString)!)
    player?.isMuted = true

    try? audioSession.setCategory(AVAudioSessionCategoryAmbient)
    try? audioSession.setActive(true)

    playerLayer = AVPlayerLayer(player: self.player)
    playerLayer?.videoGravity = .resizeAspectFill
    playerLayer?.frame = self.hlsFrameView.bounds

    hlsFrameView.layer.addSublayer(playerLayer!)
    hlsFrameView.playerLayer = playerLayer

    player!.play()
}

func killPlayer() {
    // set player to nil, otherwise it's continuing download stream
    player?.pause()
    player?.replaceCurrentItem(with: nil)
    player = nil
}

func getVotes(for carwashId: String) {
    FirebaseServices.shared.REF_VOTES.observe(.value) { (snapshot) in
        guard let snapshots = snapshot.children.allObjects as? [DataSnapshot] else { return }

        for snap in snapshots {
            guard let voteDict = snap.value as? [String: AnyObject] else { return }

            if voteDict["carwashID"] as? String == carwashId {
                let voteNumber = voteDict["votes"] as! Int
                self.voteNumberLbl.text = "\(voteNumber)"
            }
        }
    }
}

func setVote(for carwashId: String) {
    FirebaseServices.shared.REF_VOTES.child(carwashId).observeSingleEvent(of: .value) { (snapshot) in

        guard let voteDict = snapshot.value as? [String: AnyObject] else { return }

        var voteNumber = voteDict["votes"] as! Int
        voteNumber += 1
        self.voteNumberLbl.text = "\(voteNumber)"

        let auth = Auth.auth()

        auth.signInAnonymously(completion: { (user, error) in
            if error == nil {
                FirebaseServices.shared.REF_VOTES.child(carwashId).updateChildValues(["votes" : voteNumber])

            } else {
                print(error ?? "error")
            }
        })

        let user = auth.currentUser

        do {
            try auth.signOut()
            user?.delete(completion: nil)

        } catch let error as NSError {
            print(error.debugDescription)
        }
    }
}

func disableVoteButton() {
    voteButton.backgroundColor = DISABLED_VOTE_BUTTON
    voteButton.isEnabled = false
    voteButton.setTitle("Ваш голос учтен!", for: .disabled)
}

func enableVoteButton() {
    voteButton.backgroundColor = UIColor(red: 0/255, green: 188/255, blue: 212/255, alpha: 1.0)
    voteButton.isEnabled = true
    voteButton.setTitle("Хочу видеть очередь!", for: .normal)
}

@IBAction func showOnMapBtnPressed(_ sender: AnyObject) {
    guard let mapVC = UIStoryboard(name: SB_MAIN,
                                   bundle: nil).instantiateViewController(withIdentifier:
                                   VC_MAP) as? MapVC else { return }
    mapVC.cw = cw
    mainListVC.navigationController?.present(mapVC, animated: true, completion: nil)
}

@IBAction func voteButtonTapped(_ sender: Any) {
    if UserDefaults.standard.value(forKey: cw.cwID) != nil {
        disableVoteButton()

    } else {
        setVote(for: cw.cwID)
        UserDefaults.standard.set(true, forKey: cw.cwID)
        disableVoteButton()
    }
}
}

Answer 1

here just my ideas how to improve your code. maybe in some points i am wrong, but this are the places i would dig into and make it a little better:

1) set Image only once

favImg.image = UIImage(named: "icBookmarkAdded.png")

you only need to set it once - when the cell is created and not always when it prepared for each row

2) wrong nesting

if camArray != nil {
    for item in camArray! {
        if cw.cwCameraID == item["stream_id"].string {
           ...
        } else if cw.cwCameraID.isEmpty {

here you mixed the loop camArray with wrong inner if. i would test if cw.cwCameraID.isEmpty at first and later do the loop and not inside the loop otherwise this if block runs camArray.count times (also you call getVotes(for: cw.cwID) the same amount with an call to the external Firebase service in getVotes.

you should check your method

3) variable type for camArray

i would return from return VeedoServices.shared.veedoCamerasArray() an dictinary so you can get the stream directly with camMap[cw.cwCameraID] ( https://developer.apple.com/documentation/swift/dictionary ) and don't need to loop over an array

3) wrong place for camArray

i would not populate this list in every instance of the cell. an cell should be dumb as possible and i would set it from the table controller the value in configCell and there only the . otherwise it hit the VeedoServices call everytime again (i assume it is a call to a external service?)

4) on/off images

the cameraStatusImg.image = UIImage(named: "icCameraOn.png") i would avoid with placing 2 images in the cell and just hide the wrong image and unhide the right image instead of assinging a new image in every cycle

5) gettingImgLbl

i not understand why you show so many different informations to the gettingImgLbl - but as i see if there is no info goes to this label, it is possible that there can be an old text from the previous reused cw with this cell.

6) rating switch / case

  switch rating {

    case 0.0..<0.1: ratingLbl.text = "0.5"

i would calculate it direct:

  ratingLbl.text = = String(((rating * 10).rounded(.down) + 1) * 0.5)

7) distance

        if cw.nearestCW() == 0.0 {
            bottomInformationLbl.text = "Вы на месте!"

        } else {
            bottomInformationLbl.text = String(format: "Расстояние до мойки %.1f км", cw.nearestCW() / 1000)
        }

i guess that cw.nearestCW() gives you the distance in meters. but what is if it is beetween >0 and <100, then this text shows "Расстояние до мойки 0.0 км"?

but anyhow i would show up to 1km the exact meters and over this the km

8) Firebase

    FirebaseServices.shared.REF_VOTES.observe(.value) { (snapshot) in
    guard let snapshots = snapshot.children.allObjects as? [DataSnapshot] else { return }

    for snap in snapshots {
        guard let voteDict = snap.value as? [String: AnyObject] else { return }

        if voteDict["carwashID"] as? String == carwashId {

i don't know firebase not well but i think to loop over an array is too much in this place (you need also to fetch all the snapshots here. try to get only the carwash like you do in your next method FirebaseServices.shared.REF_VOTES.child(carwashId)

9) set only once

voteButton.setTitle("Хочу видеть очередь!", for: .normal)

you name a voting button "i like to see the queue"? i think this is wrong text for a voting. clickbait?

10) wrong place for disableVoteButton

            if UserDefaults.standard.value(forKey: cw.cwID) != nil {
                disableVoteButton()
            } else {
                enableVoteButton()
            }

why this is inside at a very nested if block? it should be run always