0

In one of my view controllers, I'm using PHPickerViewController. When I get my results, I then upload the URL to Firebase. It works fine, but with this line of code, try! FileManager.default.copyItem(at: videoURL, to: copiedURLFile), I'm not sure if I'm doubling the amount of time it takes for the video to upload. Honestly, I saw this line in a tutorial and added it absentmindedly, is it really necessary, and if not, will it save me time (especially since I sometimes get errors for longer videos)?

result.itemProvider.loadFileRepresentation(forTypeIdentifier: UTType.movie.identifier)
                { videoURL, error in
                    
                    assert(Thread.isMainThread == false)

                    let directory = NSTemporaryDirectory()
                    let fileName = NSUUID().uuidString.appending(".mov")
                    
                    if let videoURL = videoURL,
                       let copiedURLFile = NSURL.fileURL(withPathComponents: [directory, fileName]) {
                        try! FileManager.default.copyItem(at: videoURL, to: copiedURLFile)
                        DispatchQueue.main.async {
                            observer.hasUploaded = true
                            observer.uploadVideo(videoURL: copiedURLFile, uid: self.uid!, fileID: self.fileID, progressWheel: self.progressWheel, errorLabel: self.errorLabel, progressView: self.progressView, progressLabel: self.progressLabel)
                            
                            print("uploaded to the cloud")
                        }
                    }
                    else { print(error!); print("video might be too long") /*alert*/}
            }
EAO123
  • 55
  • 9
  • 1
    there are many things wrong with this code, "doubling the time" is least of your concerns I would say. For example if `uploadVideo` really uploads the video, then it shouldn't be on main thread; `copyItem` just duplicates the file, so you are wasting user's hard disk space; you are using old `NS` syntax - e.g. should be `UUID()`, not `NSUUID()`; too many force unwraps; `assert` is not a good way to ensure you are not running on main thread in production code... – timbre timbre Aug 31 '22 at 13:58
  • Yikes! my code doesn't work if i don't copy the item. I'll make sure to delete the file after it uploads to Firebase. Also, all variables that are force unwrapped will never be nil. How do you I suggest I replace the ```assert```? – EAO123 Aug 31 '22 at 15:31
  • 1
    `try! FileManager.default.copyItem` is not guaranteed to succeed and may crash your app. So is `error!`. And `self.uid!` is only guaranteed to be nil by your current knowledge.. Use `guard ... else` pattern instead to unwrap optionals. – timbre timbre Aug 31 '22 at 16:18
  • 1
    regarding assert depends what you want to do: if you want to actively send your work away from main thread, just do that: `DispatchQueue.global.async` for example. And if you want to quit function if you are on main thread, just do that `guard !Thread.isMainThread else { return }` – timbre timbre Aug 31 '22 at 16:20
  • 1
    if you delete file you copy - fine. Just make sure you are not uploading it on main thread – timbre timbre Aug 31 '22 at 16:29
  • @sfgblackwarkrts Thank you for this feedback. I decided to move the item instead of copy the item. I will also implement these other tips. – EAO123 Sep 02 '22 at 10:29

0 Answers0