Review

README.md

@@ -1 +1 @@
-# addNote2
+# addNote2 - Тестовое задание Вадима Пустовойтова на позицию iOS разработчика

nodeProject.xcodeproj/project.pbxproj

@@ -20,14 +20,14 @@
 /* Begin PBXFileReference section */
 		5AD6B8ED211EA1CC0080EA0F /* nodeProject.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = nodeProject.app; sourceTree = BUILT_PRODUCTS_DIR; };
 		5AD6B8F0211EA1CC0080EA0F /* AppDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppDelegate.swift; sourceTree = "<group>"; };
-		5AD6B8F2211EA1CC0080EA0F /* ViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ViewController.swift; sourceTree = "<group>"; };
+		5AD6B8F2211EA1CC0080EA0F /* ViewController.swift */ = {isa = PBXFileReference; indentWidth = 4; lastKnownFileType = sourcecode.swift; path = ViewController.swift; sourceTree = "<group>"; tabWidth = 4; };
 		5AD6B8F5211EA1CC0080EA0F /* Base */ = {isa = PBXFileReference; lastKnownFileType = file.storyboard; name = Base; path = Base.lproj/Main.storyboard; sourceTree = "<group>"; };
 		5AD6B8F8211EA1CC0080EA0F /* nodeProject.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = nodeProject.xcdatamodel; sourceTree = "<group>"; };
 		5AD6B8FA211EA1CF0080EA0F /* Assets.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Assets.xcassets; sourceTree = "<group>"; };
 		5AD6B8FD211EA1CF0080EA0F /* Base */ = {isa = PBXFileReference; lastKnownFileType = file.storyboard; name = Base; path = Base.lproj/LaunchScreen.storyboard; sourceTree = "<group>"; };
 		5AD6B8FF211EA1CF0080EA0F /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
-		5AD6B928211F31670080EA0F /* addNote.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = addNote.swift; sourceTree = "<group>"; };
-		5AD6B94B2121C8880080EA0F /* ImageViewCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageViewCell.swift; sourceTree = "<group>"; };
+		5AD6B928211F31670080EA0F /* addNote.swift */ = {isa = PBXFileReference; indentWidth = 4; lastKnownFileType = sourcecode.swift; path = addNote.swift; sourceTree = "<group>"; tabWidth = 4; };
+		5AD6B94B2121C8880080EA0F /* ImageViewCell.swift */ = {isa = PBXFileReference; indentWidth = 4; lastKnownFileType = sourcecode.swift; path = ImageViewCell.swift; sourceTree = "<group>"; tabWidth = 4; };
 /* End PBXFileReference section */
 
 /* Begin PBXFrameworksBuildPhase section */

nodeProject.xcodeproj/xcuserdata/andreytchernov.xcuserdatad/xcschemes/xcschememanagement.plist

@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>SchemeUserState</key>
+	<dict>
+		<key>nodeProject.xcscheme</key>
+		<dict>
+			<key>orderHint</key>
+			<integer>0</integer>
+		</dict>
+	</dict>
+</dict>
+</plist>

nodeProject/ImageViewCell.swift

@@ -14,20 +14,21 @@ class ImageViewCell: UICollectionViewCell {
 
     var index: NSInteger = 0
 
-    var delegate: ImageViewCellDelegate? = nil
+    //Review: делегаты должны быть по слабым ссылкам
+    /* weak */ var delegate: ImageViewCellDelegate? = nil
 
     @IBAction func removeImage(_ sender: Any) {
+        //Review: Проверка на nil не нужна
         if( delegate != nil ) {
             delegate?.removePhoto(index: index)
         }
     }
 
+    //Review: Методы подразумевающие действий называть нужно глаголами - hide()
     func hidden() {
-
     }
 
     func show() {
-
     }
 }
 protocol ImageViewCellDelegate {

nodeProject/ViewController.swift

@@ -9,14 +9,18 @@
 import UIKit
 import CoreData
 
+//Review: Именование класса. Непонятно что делает конкретно этот контроллер
 class ViewController: UIViewController, UITableViewDataSource, UITableViewDelegate, addNoteDelegate {
 
     @IBAction func addNoteForm(_ sender: Any) {
+        //Review: Навигацию лучше через Storyboard делать
         let vc2 = storyboard?.instantiateViewController(withIdentifier: "addNoteSID") as! addNote
         vc2.delegate = self
         navigationController?.pushViewController(vc2, animated: true)
     }
+
     @IBAction func deleteAll(_ sender: Any) {
+        //Review: Вынести методы работы с данными в отдельный класс
         let context = (UIApplication.shared.delegate as! AppDelegate).persistentContainer.viewContext
 
         for name in names {
@@ -31,20 +35,21 @@ class ViewController: UIViewController, UITableViewDataSource, UITableViewDelega
 
         tableView.reloadData()
     }
-
+    //Review: private
     @IBOutlet weak var tableView: UITableView!
 
     var names = [Nodes]()
 
     override func viewDidLoad() {
         super.viewDidLoad()
-
+        //Review: Делается в Storyboard
         title = "\"The List\""
         tableView.register(UITableViewCell.self, forCellReuseIdentifier: "Cell")
         tableView.delegate = self
         tableView.dataSource = self
     }
 
+    //Review: Вынести методы делегатов и источников данных в отдельный класс/extension к контроллеру
     func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
         return names.count
     }
@@ -57,9 +62,11 @@ class ViewController: UIViewController, UITableViewDataSource, UITableViewDelega
     }
 
     func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
-
+        //Review: Простую навигацию лучше через Storyboard, передача параметров в методе prepareToSegue
         let vc2 = storyboard?.instantiateViewController(withIdentifier: "addNoteSID") as! addNote
         navigationController?.pushViewController(vc2, animated: true)
+
+        //Review: Использование force-cast'ов
         vc2.titl = names[indexPath.row].name!
         vc2.info = (names[indexPath.row].info)!
         let adv: Int = (names[indexPath.row].picturesN?.count)!
@@ -94,7 +101,7 @@ class ViewController: UIViewController, UITableViewDataSource, UITableViewDelega
 
         readData()
     }
-
+    //Review: Ненужный код
     override func didReceiveMemoryWarning() {
         super.didReceiveMemoryWarning()
         // Dispose of any resources that can be recreated.
@@ -107,6 +114,7 @@ class ViewController: UIViewController, UITableViewDataSource, UITableViewDelega
 
     }
 
+    //Review: Вынести в класс работы с базой данных
     func readData(){
         let context = (UIApplication.shared.delegate as! AppDelegate).persistentContainer.viewContext
 

nodeProject/addNote.swift

@@ -8,38 +8,56 @@
 
 import UIKit
 
+//Review: Code-style (https://github.com/raywenderlich/swift-style-guide) - Классы с заглавной буквы. Для наследников от Cocoa-классов имя должно заканчиваться на базовый класс. Т.е. AddNoteViewController
 class addNote: UIViewController, UIImagePickerControllerDelegate,UINavigationControllerDelegate, UICollectionViewDelegate, UICollectionViewDataSource, ImageViewCellDelegate {
 
+    //Review: Должны быть private, должны быть в MARK секции (код контроллера на несколько страниц)
     @IBOutlet weak var nameNoteText: UITextField!
     @IBOutlet weak var infoNoteText: UITextView!
+    //Review: Опечатка
     @IBOutlet weak var colleectionViewImages: UICollectionView!
     @IBOutlet weak var begDateText: UITextField!
     @IBOutlet weak var updateDateText: UITextField!
 
+    //Review: Те поля что дальше должны быть собраны в модель заметки, инициализированной по умолчанию, чтобы не захламлять контроллер и оперировать уже этой моделью
+
+    //Review: Должны быть приватными хотя бы сеттеры - private (set)
     var names = [Nodes]()
     var picturesNode = [Pictures]()
+    //Review: Code-style, именование переменных
     var titl :String = ""
     var info = ""
+    //Review: Должен быть enum тип
     var type = 0
-    var picture = UIImage()
+    //Review: Зачем пустое изображение?, пусть будет неинициализировано
+    var picture = UIImage() //
     let imagePicker =  UIImagePickerController()
     var arrayImage: [UIImage] = [UIImage]()
-    var delegate: addNoteDelegate? = nil
+    //Review: Делегаты должны храниться по weak ссылкам (https://medium.com/@JoyceMatos/arc-strong-and-weak-references-in-swift-f2a085a17119, https://docs.swift.org/swift-book/LanguageGuide/AutomaticReferenceCounting.html)
+    /* weak */ var delegate: addNoteDelegate? = nil
+
+    //Review: Должен быть Bool тип
     var isEdit = 0
 
+
+    //Review: методы которые используются только кнутри класса должны быть private
     @IBAction func saveNote(_ sender: Any) {
+        //Review: не стоит использовать force cast (!), лучше заменить на (nameNoteText.text ?? "")
         self.saveName(name: nameNoteText.text!, info: infoNoteText.text, pictures: arrayImage)
         delegate?.updateTable()
         navigationController?.popViewController(animated: true)
     }
+
     override func viewDidLoad() {
         super.viewDidLoad()
 
+        //Review: Хоть что-то кинуть в NSLog в обратном случае
         if( UIImagePickerController.isSourceTypeAvailable(.photoLibrary)) {
             imagePicker.delegate = self
             imagePicker.sourceType = .photoLibrary
         }
 
+
         colleectionViewImages.dataSource = self
         colleectionViewImages.delegate = self
 
@@ -48,18 +66,21 @@ class addNote: UIViewController, UIImagePickerControllerDelegate,UINavigationCon
         }
     }
 
+    //Review: Ненужный код
     override func didReceiveMemoryWarning() {
         super.didReceiveMemoryWarning()
         // Dispose of any resources that can be recreated.
     }
 
-    func saveName(name: String, info: String, pictures: [UIImage]){
+    func saveName(name: String, info: String, pictures: [UIImage]) {
+        //Review: Работу с базой данных надо выносить в отдельный класс
         let context = (UIApplication.shared.delegate as! AppDelegate).persistentContainer.viewContext
         let node = Nodes(entity: Nodes.entity(), insertInto: context)
         let pictureSave = Pictures(entity: Pictures.entity(), insertInto: context)
 
         node.setValue(name, forKey: "name")
         node.setValue(info, forKey: "info")
+        //Review: Code-style
         if isEdit == 0
         {
          node.setValue(NSDate(), forKey: "begDate")
@@ -70,6 +91,15 @@ class addNote: UIViewController, UIImagePickerControllerDelegate,UINavigationCon
         }
 
 //        let pictureCount = pictures.count // цикл по колличеству картинок если разберусь с таблицей!!!
+
+        //Review: Лучше так
+        /*
+        for picture in pictures {
+            let pictureN = UIImageJPEGRepresentation(picture, 0.0)
+            pictureSave.setValue(pictureN, forKey: "picture")
+            node.addToPicturesN(pictureSave)
+        }
+        */
         if( pictures.count > 0 ) {
             for index in 0...pictures.count - 1 {
                 let pictureN = UIImageJPEGRepresentation(pictures[index], 0.0)
@@ -87,6 +117,7 @@ class addNote: UIViewController, UIImagePickerControllerDelegate,UINavigationCon
         }
     }
 
+    //Review: Плохо выделять функцию ради одной строчки
     func presentImagePicker() {
         present(imagePicker, animated: true, completion: nil)
     }
@@ -140,14 +171,25 @@ class addNote: UIViewController, UIImagePickerControllerDelegate,UINavigationCon
         if( indexPath.row == arrayImage.count) {
             presentImagePicker()
         }
+        //Review: Добавить сюда снятие выделения
+        //collectionView.deselectItem(at: indexPath, animated: false)
     }
 
+    //Review: Зачем на Deselect?
     func collectionView(_ collectionView: UICollectionView, didDeselectItemAt indexPath: IndexPath) {
         if( indexPath.row == arrayImage.count) {
             presentImagePicker()
         }
     }
 }
+//Review: Если не получается вынести иточник данных и делегат в отдельный класс (хотя в этом случае можно было бы), то лучше делать вот так:
+/*
+ extension addNote: UICollectionViewDelegate {
+    ...
+}
+ */
+
+
 
 protocol addNoteDelegate {
     func updateTable()