Aide pour corriger et améliorer mon code

#1

Bonjour à tous !

J’aimerais avoir l’avis de développeur pour corriger mon code ou bien savoir ce que vous vous auriez fait afin d’améliorer l’utilisation de la memoire ou bien la lisibilité de mon code ou bien n’importe quoi d’autre !

En gros, je veux du feedback !

Mon application est une version “améliorée” de l’app Vision du cours !

J’ai un bouton qui permet de mettre une liste de mot dans des labels et ensuite je compare le mot “bestGuess” a ma liste de label. Si ca match, le label devient vert.

Voila le code :

class ViewController: UIViewController, AVCaptureVideoDataOutputSampleBufferDelegate {

    lazy var imageRecognitionRequest: VNRequest = {
        let model = try! VNCoreMLModel(for: Inceptionv3().model)
        let request = VNCoreMLRequest(model: model, completionHandler: self.imageRecognitionHandler)
        return request
    }()
    
    func imageRecognitionHandler(request: VNRequest, error:Error?) {
        guard let observations = request.results as? [VNClassificationObservation],
            let bestGuess = observations.first else {
                return
        }
        
        var finalIdentifier = bestGuess.identifier
        var indexComma = finalIdentifier.firstIndex(of: ",") ?? finalIdentifier.endIndex
        finalIdentifier = String(finalIdentifier[..<indexComma])
        
        var bestGuessNice = String(bestGuess.confidence*100)
        var indexDigit = bestGuessNice.firstIndex(of: ".")
        var oneDigit = bestGuessNice.index(after: indexDigit!)
        bestGuessNice = String(bestGuessNice[...oneDigit])
                    
        DispatchQueue.main.async {
            if bestGuess.confidence > 0.2 {
                self.ui_titlelabel.text = finalIdentifier
                self.ui_infolabel.text = "Probabilité: \(bestGuessNice) %"
            } else {
                self.ui_infolabel.text = "Recherche en cours..."
            }
            if self.listWordsToCompare.contains(finalIdentifier) == true {
                let wordFound: String = finalIdentifier
                print(wordFound)
                
                switch wordFound {
                case self.listWordsToCompare[0]:
                    if self.listWordsToCompare[0] == wordFound {
                        self.ui_listWorld_00.backgroundColor = .green
                        print("Mot 1 trouve !")
                    }
                case self.listWordsToCompare[1]:
                    if self.listWordsToCompare[1] == wordFound {
                        self.ui_listWorld_01.backgroundColor = .green
                        print("Mot 2 trouve !")
                    }
                case self.listWordsToCompare[2]:
                    if self.listWordsToCompare[2] == wordFound {
                        self.ui_listWorld_02.backgroundColor = .green
                        print("Mot 3 trouve !")
                    }
                case self.listWordsToCompare[3]:
                    if self.listWordsToCompare[3] == wordFound {
                       self.ui_listWorld_03.backgroundColor = .green
                        print("Mot 4 trouve !")
                    }
                case self.listWordsToCompare[4]:
                    if self.listWordsToCompare[4] == wordFound {
                        self.ui_listWorld_04.backgroundColor = .green
                        print("Mot 5 trouve !")
                    }
                case self.listWordsToCompare[5]:
                    if self.listWordsToCompare[5] == wordFound {
                        self.ui_listWorld_05.backgroundColor = .green
                        print("Mot 6 trouve !")
                    }
                case self.listWordsToCompare[6]:
                    if self.listWordsToCompare[6] == wordFound {
                        self.ui_listWorld_06.backgroundColor = .green
                        print("Mot 7 trouve !")
                    }
                case self.listWordsToCompare[7]:
                    if self.listWordsToCompare[7] == wordFound {
                        self.ui_listWorld_07.backgroundColor = .green
                            print("Mot 8 trouve !")
                    }
                case self.listWordsToCompare[8]:
                    if self.listWordsToCompare[8] == wordFound {
                       self.ui_listWorld_08.backgroundColor = .green
                        print("Mot 9 trouve !")
                    }
                case self.listWordsToCompare[9]:
                    if self.listWordsToCompare[9] == wordFound {
                       self.ui_listWorld_09.backgroundColor = .green
                        print("Mot 10 trouve !")
                    }
                default:
                    print("breaaaaak")
                    break
                }
            }
        }
    }
    
    let captureSession = AVCaptureSession()
    @IBOutlet var ui_preview: UIView!
    @IBOutlet var ui_titlelabel: UILabel!
    @IBOutlet var ui_infolabel: UILabel!
    
    @IBAction func startRecord(_ sender: Any) {
        if captureSession.isRunning {
            captureSession.stopRunning()
        } else {
            if captureSession.inputs.count == 0 {
                configureCaptureSession()
            }
            captureSession.startRunning()
        }
    }

    override func viewDidLoad() {
        super.viewDidLoad()
        // Do any additional setup after loading the view.
    }
    
    func configureCaptureSession() {
        if let camera = AVCaptureDevice.default(for: AVMediaType.video),
            let cameraFeed = try? AVCaptureDeviceInput(device: camera) {
            captureSession.addInput(cameraFeed)
            let outputFeed = AVCaptureVideoDataOutput()
            outputFeed.setSampleBufferDelegate(self, queue: DispatchQueue.global(qos: DispatchQoS.QoSClass.userInitiated))
            captureSession.addOutput(outputFeed)
            let previewLayer = AVCaptureVideoPreviewLayer(session: captureSession)
            previewLayer.frame = ui_preview.bounds
            ui_preview.layer.addSublayer(previewLayer)
        }
    }
    func captureOutput(_ output: AVCaptureOutput, didOutput sampleBuffer: CMSampleBuffer, from connection: AVCaptureConnection) {
        guard let pixelBuffer = CMSampleBufferGetImageBuffer(sampleBuffer) else {
            return
        }
        let imageRequestHandler = VNImageRequestHandler(cvPixelBuffer: pixelBuffer, orientation: CGImagePropertyOrientation.up, options: [:])
        do { try imageRequestHandler.perform([imageRecognitionRequest]) }
        catch { print(error) }
    }
    func captureOutput(_ output: AVCaptureOutput, didDrop sampleBuffer: CMSampleBuffer, from connection: AVCaptureConnection) {
    }
    
    
    
    // GAME ================================================================================
    
    @IBOutlet var ui_listWorld_00: UILabel!
    @IBOutlet var ui_listWorld_01: UILabel!
    @IBOutlet var ui_listWorld_02: UILabel!
    @IBOutlet var ui_listWorld_03: UILabel!
    @IBOutlet var ui_listWorld_04: UILabel!
    @IBOutlet var ui_listWorld_05: UILabel!
    @IBOutlet var ui_listWorld_06: UILabel!
    @IBOutlet var ui_listWorld_07: UILabel!
    @IBOutlet var ui_listWorld_08: UILabel!
    @IBOutlet var ui_listWorld_09: UILabel!
    @IBOutlet var ui_view_worldList: UIView!
    
    let allTheWords = ["pen", "screen", "computer keyboard", "cup", "door", "mouse", "monitor", "notebook", "computer", "laptop", "soap", "chair", "spoon"]
    var soundGame = AVAudioPlayer()
    var listWordsToCompare = Array<String>()
    
    
    @IBAction func kadocHeadTouched(_ sender: Any) {
        
        let sonKadoc = Bundle.main.path(forResource: "a_kadoc", ofType: "mp3")
        do { try soundGame = AVAudioPlayer(contentsOf: URL(fileURLWithPath: sonKadoc!)) }
        catch { print("whathappened") }
        //soundGame.play()
        newListOfWordsIntoLabel()
    }
 
    func newListOfWordsIntoLabel() {
        let numberQuestions = allTheWords.count
        let indexPickQuestions = numberQuestions - 10
        let randomInt = Int.random(in: 0...indexPickQuestions)
        let wordsPickedUp = allTheWords[randomInt..<Int(randomInt + 10)]
        
        let newList = Array(wordsPickedUp)
        listWordsToCompare = newList
        print(newList)
        print(listWordsToCompare)
        
        ui_listWorld_00.text = newList[0]
        ui_listWorld_01.text = newList[1]
        ui_listWorld_02.text = newList[2]
        ui_listWorld_03.text = newList[3]
        ui_listWorld_04.text = newList[4]
        ui_listWorld_05.text = newList[5]
        ui_listWorld_06.text = newList[6]
        ui_listWorld_07.text = newList[7]
        ui_listWorld_08.text = newList[8]
        ui_listWorld_09.text = newList[9]
        
        ui_listWorld_00.backgroundColor = .red
        ui_listWorld_01.backgroundColor = .red
        ui_listWorld_02.backgroundColor = .red
        ui_listWorld_03.backgroundColor = .red
        ui_listWorld_04.backgroundColor = .red
        ui_listWorld_05.backgroundColor = .red
        ui_listWorld_06.backgroundColor = .red
        ui_listWorld_07.backgroundColor = .red
        ui_listWorld_08.backgroundColor = .red
        ui_listWorld_09.backgroundColor = .red
    }

}

Edit :

Vous n’allez pas me faire croire que c’est parfait :smiley:
Ca fait quelque mois que j’essaye de vraiment développer correctement mais je manque vraiment de base, donc n’hésitez pas.
Merci à ceux qui prendront du temps pour répondre !

#2

Hello @Nicow

Dans ton code, tu as laissé la fonction viewDidLoad, sans l’utiliser, donc pas besoin de l’écrire:

override func viewDidLoad() {
    super.viewDidLoad()
    // Do any additional setup after loading the view.
}

Dans ta fonction configureCaptureSession(), tu utilises un if let pour deux variables dont tu as absolument besoin dans la suite de ta fonction:

if let camera = AVCaptureDevice.default(for: AVMediaType.video),
    let cameraFeed = try? AVCaptureDeviceInput(device: camera) {

Au lieu de faire ça, personnellement, je ferai plutôt ceci:

guard let camera = AVCaptureDevice.default(for: AVMediaType.video),
    let cameraFeed = try? AVCaptureDeviceInput(device: camera) else {
    return
}
        
captureSession.addInput(cameraFeed)

let outputFeed = AVCaptureVideoDataOutput()
outputFeed.setSampleBufferDelegate(self, queue: DispatchQueue.global(qos: DispatchQoS.QoSClass.userInitiated))
captureSession.addOutput(outputFeed)

let previewLayer = AVCaptureVideoPreviewLayer(session: captureSession)
previewLayer.frame = ui_preview.bounds
ui_preview.layer.addSublayer(previewLayer)

Ca t’évite d’avoir un niveau d’indentation supplémentaire.
Aussi, je n’hésiterai pas à espacer un peu le code (comme je l’ai fais juste au dessus, je trouve que c’est plus lisible).

Tu peux même le faire de manière générale (je pense notamment à ce genre de code):

func maFonction() {

}
// Un petit retour à la ligne ici ne fait pas de mal, plutôt que de tout coller.
func maDeuxiemeFonction() {

}

Quand tu as ce genre de chose:

class ViewController: UIViewController, AVCaptureVideoDataOutputSampleBufferDelegate {

Perso, j’aime bien utiliser une extension pour diviser mon code, comme ça:

class ViewController: UIViewController {
    // Le code de ton view controller, classique
}

extension ViewController: AVCaptureVideoDataOutputSampleBufferDelegate {
    // Le code lié au delegate
}

Au premier coup d’oeil, c’est ce qui m’est venu :wink:

Par contre, tu aurais une capture d’écran de ce que ça donne visuellement ?
Je suis pas fan de ce genre de code:

ui_listWorld_00.backgroundColor = .red
ui_listWorld_01.backgroundColor = .red
ui_listWorld_02.backgroundColor = .red
ui_listWorld_03.backgroundColor = .red
ui_listWorld_04.backgroundColor = .red
ui_listWorld_05.backgroundColor = .red
ui_listWorld_06.backgroundColor = .red
ui_listWorld_07.backgroundColor = .red
ui_listWorld_08.backgroundColor = .red
ui_listWorld_09.backgroundColor = .red

Peut-être qu’il y a moyen de faire autrement ?

J’espère que ça t’aidera,

Bonne journée,

Alexandre

#3

Salut !

Merci beaucoup pour ta réponse !
Je ne savais pas qu’une extension servais à cela, je vais essayer de m’en servir plus souvent.

Par rapport a ca :

ui_listWorld_00.backgroundColor = .red
ui_listWorld_01.backgroundColor = .red
ui_listWorld_02.backgroundColor = .red
ui_listWorld_03.backgroundColor = .red
ui_listWorld_04.backgroundColor = .red
ui_listWorld_05.backgroundColor = .red
ui_listWorld_06.backgroundColor = .red
ui_listWorld_07.backgroundColor = .red
ui_listWorld_08.backgroundColor = .red
ui_listWorld_09.backgroundColor = .red

J’ai essayer de raccourcir mon code mais d’après ce que jai compris, un outlet pointe a un endroit spécifique de la memoire et donc on ne peut pas rassembler plusieurs outlets dans un array pour tous les modifier en même temps.

Et voilà un screen pour que tu puisses apprécier mes talents de graphiste des années Windows 95 ! :smiley:

Encore merci,
Salut.

Edit : avec le screen, c’est mieux.