1
\$\begingroup\$

im trying to migrate my very large swift view controller to mvvm but it still feels very large, can you guys give me any advice

What this controller does is simple it shows a UIView on which i can draw the buttons that I display are here to manage the lines that i have drawn on the canvas.

For example to delete the last line drawn, delete all lines, change the color etc ...

What I want to do is to create my buttons while conforming to MVVM, so its not as ugly as my view controller is right now

@objc class VideoEditorViewController: UIViewController {
    
    let canvas = Canvas()
    let picker = UIColorPickerViewController()
    
    
    func setCanvasUI() {
        self.view.addSubview(canvas)
        canvas.backgroundColor = .clear
        let undoButton = CanvasButtonsViewModel(frame: .zero)
        undoButton.configure(with: CanvasModelButtonModel(image:
                                                        UIImage(systemName: "arrowshape.turn.up.left.fill")?
                                                        .withTintColor(.white, renderingMode: .alwaysOriginal), width: 50, height: 50, backgroundColor: .white))
        undoButton.addTarget(self, action: #selector(handleUndo), for: .touchUpInside)
        let colorPicker = CanvasButtonsViewModel(frame: .zero)
        colorPicker.configure(with: CanvasModelButtonModel(image:
                                                        UIImage(systemName: "pencil.circle")?
                                                        .withTintColor(.white, renderingMode: .alwaysOriginal), width: 50, height: 50, backgroundColor: .white))
        colorPicker.addTarget(self, action: #selector(ColorPicker), for: .touchUpInside)
        let trashCanButton = CanvasButtonsViewModel(frame: .zero)
        trashCanButton.configure(with: CanvasModelButtonModel(image:
                                                        UIImage(systemName: "trash")?
                                                        .withTintColor(.white, renderingMode: .alwaysOriginal), width: 50, height: 50, backgroundColor: .white))
        trashCanButton.addTarget(self, action: #selector(handleClear), for: .touchUpInside)
        let uploadViewButton = CanvasButtonsViewModel(frame: .zero)
        uploadViewButton.configure(with: CanvasModelButtonModel(image:
                                                                    UIImage(systemName: "envelope")?
                                                                    .withTintColor(.white, renderingMode: .alwaysOriginal), width: 51, height: 48, backgroundColor: .white))
        let test = CanvasButtonsViewModel(frame: .zero)
        test.configure(with: CanvasModelButtonModel(image:
                                                                    UIImage(systemName: "pencil.circle")?
                                                                    .withTintColor(.white, renderingMode: .alwaysOriginal), width: 51, height: 48, backgroundColor: .white))
        test.addTarget(self, action: #selector(testdraw), for: .touchUpInside)
        let stackView = UIStackView(arrangedSubviews: [
            undoButton,
            trashCanButton,
            colorPicker,
            uploadViewButton,
            test
            
        ])
        view.addSubview(stackView)
        stackView.axis = .vertical
        stackView.bringSubviewToFront(self.view)
        stackView.spacing = 30
        stackView.snp.makeConstraints { (make) in
            make.right.equalTo(view.snp_rightMargin).offset(-20)
            make.top.equalTo(view.snp_topMargin)
        }
        canvas.frame = view.frame
    }
    
    override func viewDidAppear(_ animated: Bool) {
        super.viewDidAppear(animated)
        setCanvasUI()
    }
    
    override func viewDidLoad() {
        super.viewDidLoad()
    }
}


extension VideoEditorViewController: UIColorPickerViewControllerDelegate, UIImagePickerControllerDelegate {
    @objc func handleUndo() {
        canvas.undo()
    }
    @objc func handleClear() {
        canvas.undoAll()
    }
    @objc func ColorPicker() {
        present(picker, animated: true, completion: nil)
        canvas.setStrokeColor(color: picker.selectedColor)
    }
    @objc func testdraw() {
        if (canvas.isDrawable == false) {
            canvas.setDrawable(state: true)
            return
        }
        if (canvas.isDrawable == true) {
            canvas.setDrawable(state: false)
            return
        }
    }
}

Canvas View :

import Foundation
import UIKit
import SnapKit

class Canvas: UIView {
    
    private var strokeColor = UIColor.black
    private var strokeWidth: Float = 10
    public var isDrawable: Bool = false
    
    public func setStrokeColor(color: UIColor) {
        self.strokeColor = color
    }
    
    public func setDrawable(state: Bool) {
        self.isDrawable = state
    }
    
    public func undo() {
        _ = lines.popLast()
        setNeedsDisplay()
    }
    public func undoAll() {
        lines.removeAll()
        setNeedsDisplay()
    }
    
    public func changeWidth(width: Float) {
        self.strokeWidth = width
        setNeedsDisplay()
    }
    public func draw() {
        guard let context = UIGraphicsGetCurrentContext() else { return }
    
        lines.forEach { (line) in
            context.setStrokeColor(line.color.cgColor)
            context.setLineWidth(CGFloat(line.strokeWidth))
            context.setLineCap(.round)
        for(i, p) in line.points.enumerated() {
                if (i == 0) {
                    context.move(to: p)
                } else {
                    context.addLine(to: p)
                }
            }
            context.strokePath()
        }
    }
    
    override func draw(_ rect: CGRect) {
        super.draw(rect)
        if (isDrawable == true) {
            draw()
        }
    }
    var lines = [Line]()
    
    
    override func touchesBegan(_ touches: Set<UITouch>, with event: UIEvent?) {
        lines.append(Line.init(strokeWidth: strokeWidth, color: strokeColor, points: [], drawable: isDrawable))
    }
    
    override func touchesMoved(_ touches: Set<UITouch>, with event: UIEvent?) {
        if (isDrawable == true) {
            guard let point = touches.first?.location(in: nil) else { return }
        
            guard var lastLine = lines.popLast() else { return }
            lastLine.points.append(point)
            lines.append(lastLine)
            print(point)
            setNeedsDisplay()
        }
    }
}

Canvas Struct to draw lines :

struct Line {
    let strokeWidth: Float
    let color: UIColor
    var points: [CGPoint]
    var drawable: Bool
}

My view model

final class CanvasButtonsViewModel: UIButton {
    
    let image: UIImageView = {
        let image = UIImageView()
        image.image = UIImage()
        image.image?.withTintColor(.white, renderingMode: .alwaysOriginal)
        return image
    }()

    func setConfig(size: CGFloat) -> UIImage.SymbolConfiguration {
        let largeConfig = UIImage.SymbolConfiguration(pointSize: size, weight: .bold, scale: .large)
        return largeConfig
    }
    
    override init(frame: CGRect) {
        super.init(frame: frame)
    }
    required init?(coder: NSCoder) {
        fatalError("init(coder:) has not been implemented")
    }
    func configure(with viewModel: CanvasModelButtonModel) {
        self.addSubview(image)
        image.image = viewModel.image
        image.frame = CGRect(x: 0, y: 0, width: viewModel.width, height: viewModel.height)
    }
    override func layoutSubviews() {
        super.layoutSubviews()
    }
}

The model

struct CanvasModelButtonModel {
    let image: UIImage?
    let width: CGFloat
    let height: CGFloat
    let backgroundColor: UIColor?
}
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Welcome to Code Review! What task does this code accomplish? Please tell us, and also make that the title of the question via edit. Maybe you missed the placeholder on the title element: "State the task that your code accomplishes. Make your title distinctive.". Also from How to Ask: "State what your code does in your title, not your main concerns about it.". \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Nov 20 '20 at 16:24
0
\$\begingroup\$
// General Comments
/**
 - All view model should not be inherited from UIView nor UIViewController they should be their own separate classes/structs
 - The provided code is all view and UI logic which do not need view models
 */
import Foundation
import UIKit
import SnapKit

@objc
class VideoEditorViewController: UIViewController {
    let canvas = Canvas()
    let picker = UIColorPickerViewController()
    
    // Life cycle functions should be at the top of the view controller after the variables and outlets
    override func viewDidAppear(_ animated: Bool) {
        super.viewDidAppear(animated)
        self.setCanvasUI()
    }
    // no need to have this function as it only calls its parent function
    override func viewDidLoad() {
        super.viewDidLoad()
    }
    
    func setCanvasUI() {
        self.view.addSubview(self.canvas)
        self.canvas.backgroundColor = .clear
        // You should seperate between each UIView with at least one space for readability
        let undoButton = CanvasButton(frame: .zero)
        let undoButtonModel = CanvasButtonModel(image: UIImage(systemName: "arrowshape.turn.up.left.fill")?.withTintColor(.white, renderingMode: .alwaysOriginal), width: 50, height: 50, backgroundColor: .white)
        undoButton.configure(with: undoButtonModel)
        undoButton.addTarget(self, action: #selector(handleUndo), for: .touchUpInside)
        
        let colorPicker = CanvasButton(frame: .zero)
        let colorPickerModle = CanvasButtonModel(image:UIImage(systemName: "pencil.circle")?.withTintColor(.white, renderingMode: .alwaysOriginal), width: 50, height: 50, backgroundColor: .white)
        colorPicker.configure(with: colorPickerModle)
        colorPicker.addTarget(self, action: #selector(ColorPicker), for: .touchUpInside)
        
        let trashCanButton = CanvasButton(frame: .zero)
        let trashCanModel = CanvasButtonModel(image: UIImage(systemName: "trash")?.withTintColor(.white, renderingMode: .alwaysOriginal), width: 50, height: 50, backgroundColor: .white)
        trashCanButton.configure(with: trashCanModel)
        trashCanButton.addTarget(self, action: #selector(handleClear), for: .touchUpInside)
        
        let uploadViewButton = CanvasButton(frame: .zero)
        let uploadButtonModel = CanvasButtonModel(image:UIImage(systemName: "envelope")?.withTintColor(.white, renderingMode: .alwaysOriginal), width: 51, height: 48, backgroundColor: .white)
        uploadViewButton.configure(with: uploadButtonModel)
        
        let test = CanvasButton(frame: .zero)
        let testModel = CanvasButtonModel(image: UIImage(systemName: "pencil.circle")?.withTintColor(.white, renderingMode: .alwaysOriginal), width: 51, height: 48, backgroundColor: .white)
        test.configure(with: testModel)
        test.addTarget(self, action: #selector(testdraw), for: .touchUpInside)
        
        let stackView = UIStackView(arrangedSubviews: [
            undoButton,
            trashCanButton,
            colorPicker,
            uploadViewButton,
            test
        ])
        
        view.addSubview(stackView)
        
        stackView.axis = .vertical
        stackView.bringSubviewToFront(self.view)
        stackView.spacing = 30
        stackView.snp.makeConstraints { (make) in
            make.right.equalTo(view.snp_rightMargin).offset(-20)
            make.top.equalTo(view.snp_topMargin)
        }
        self.canvas.frame = view.frame
    }
}

// We should use this protocol to make sure than anyone who is using the canvas with these buttons should have these functions implemented. Although I would personally prefer to have all these buttons inside the Canvas UIView along with its functionalities
@objc
protocol CanvasButtonsDelegate {
    func handleUndo()
    func handleClear()
    func colorPicker()
    func testdraw()
}

extension VideoEditorViewController: UIColorPickerViewControllerDelegate, UIImagePickerControllerDelegate, CanvasButtonsDelegate {
    // There should be at least one space between each function declaration
    @objc func handleUndo() {
        self.canvas.undo()
    }
    
    @objc func handleClear() {
        self.canvas.undoAll()
    }
    // function name should start with lower case
    @objc func colorPicker() {
        present(self.picker, animated: true, completion: nil)
        self.canvas.setStrokeColor(color: self.picker.selectedColor)
    }
    
    @objc func testdraw() {
        if (self.canvas.isDrawable == false) {
            self.canvas.setDrawable(state: true)
            return
        }
        if (self.canvas.isDrawable == true) {
            self.canvas.setDrawable(state: false)
            return
        }
    }
}

class Canvas: UIView {
    private var strokeColor = UIColor.black
    private var strokeWidth: Float = 10
    public var isDrawable: Bool = false
    var lines = [Line]()
    
    public func setStrokeColor(color: UIColor) {
        self.strokeColor = color
    }
    
    public func setDrawable(state: Bool) {
        self.isDrawable = state
    }
    
    public func undo() {
        _ = self.lines.popLast()
        self.setNeedsDisplay()
    }
    public func undoAll() {
        self.lines.removeAll()
        self.setNeedsDisplay()
    }
    
    public func changeWidth(width: Float) {
        self.strokeWidth = width
        self.setNeedsDisplay()
    }
    
    public func draw() {
        guard let context = UIGraphicsGetCurrentContext() else { return }
    
        self.lines.forEach { (line) in
            context.setStrokeColor(line.color.cgColor)
            context.setLineWidth(CGFloat(line.strokeWidth))
            context.setLineCap(.round)
            for(i, p) in line.points.enumerated() {
                if (i == 0) {
                    context.move(to: p)
                } else {
                    context.addLine(to: p)
                }
            }
            context.strokePath()
        }
    }
    
    override func draw(_ rect: CGRect) {
        super.draw(rect)
        if (self.isDrawable == true) {
            self.draw()
        }
    }
    
    override func touchesBegan(_ touches: Set<UITouch>, with event: UIEvent?) {
        self.lines.append(Line(strokeWidth: self.strokeWidth, color: self.strokeColor, points: [], drawable: self.isDrawable))
    }
    
    override func touchesMoved(_ touches: Set<UITouch>, with event: UIEvent?) {
        if (self.isDrawable == true) {
            guard let point = touches.first?.location(in: nil) else { return }
        
            guard var lastLine = self.lines.popLast() else { return }
            lastLine.points.append(point)
            self.lines.append(lastLine)
            print(point)
            self.setNeedsDisplay()
        }
    }
}

final class CanvasButton: UIButton {
    let image: UIImageView = {
        let image = UIImageView()
        image.image = UIImage()
        image.image?.withTintColor(.white, renderingMode: .alwaysOriginal)
        return image
    }()
    
    override init(frame: CGRect) {
        super.init(frame: frame)
    }
    
    required init?(coder: NSCoder) {
        fatalError("init(coder:) has not been implemented")
    }
    
    func setConfig(size: CGFloat) -> UIImage.SymbolConfiguration {
        let largeConfig = UIImage.SymbolConfiguration(pointSize: size, weight: .bold, scale: .large)
        return largeConfig
    }
    
    func configure(with viewModel: CanvasButtonModel) {
        self.addSubview(self.image)
        self.image.image = viewModel.image
        self.image.frame = CGRect(x: 0, y: 0, width: viewModel.width, height: viewModel.height)
    }
    
    override func layoutSubviews() {
        super.layoutSubviews()
    }
}

/**
 - CanvasButtonsViewModel should be CanvasButtonModel as it doesn't reflect anything related to view model concept
 - Also it should be Button not Buttons
 - CanvasModelButtonModel is just a model a container, not a view model
 - The naming convention is not being followed here ex: CanvasModelButtonModel. It should be CanvasButtonModel
*/
struct CanvasButtonModel {
    let image: UIImage?
    let width: CGFloat
    let height: CGFloat
    let backgroundColor: UIColor?
}

struct Line {
    let strokeWidth: Float
    let color: UIColor
    var points: [CGPoint]
    var drawable: Bool
}

I'm failing to understand the reason behind writing the code in such a way so I will just provide a very simple example of MVVM pattern in iOS swift using UIKit

import Foundation
import UIKit

// just a temp way to pass errors to the UI
enum ViewModelError: Error {
    case loginFailed
}

// the list of action any UI is using the provided view model should handle
protocol ViewModelDelegate {
    func onLoginSuccess(response: NSObject)
    func onLoginFailuar(error: Error)
}

// so the view will only be able to access the available functionality of this view model and not the whole properties and functionalities
// also for easier testing
@objc
protocol ViewModelInterface {
    func login()
}

// will contain all the business and API calling logic like local validation, on network error, etc
final class ViewModel: ViewModelInterface {
    
    var delegate: ViewModelDelegate?
    
    init(delegate: ViewModelDelegate? = nil) {
        self.delegate = delegate
    }
    
    @objc func login() {
        // async call here to login the user
        if true { // success
            self.delegate?.onLoginSuccess(response: NSObject())
        } else { // failed
            self.delegate?.onLoginFailuar(error: ViewModelError.loginFailed)
        }
    }
}

// will contain all the UI logic
class ViewController: UIViewController {
    // MARK:- Outlets
    let loginButton: UIButton = UIButton()
    
    // MARK:- ViewModel
    var vm: ViewModelInterface!
    
    // MARK:- Lifecycle
    override func viewDidLoad() {
        super.viewDidLoad()
        // set the view model delegate
        self.vm = ViewModel(delegate: self)
        
        // UI is dump and should not know what to do here and it is the view model responsibility to know what to do on any kind of events
        self.loginButton.addTarget(self.vm, action: #selector(self.vm.login), for: .touchDown)
    }
}

extension ViewController: ViewModelDelegate {
    func onLoginSuccess(response: NSObject) {
        // 1. save token in the user default
        // 2. navigate to home screen
    }
    
    func onLoginFailuar(error: Error) {
        // show error on the UI
    }
}

```
\$\endgroup\$
0

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.