trjhtr

I know there is some repeated code in here, which is why I need a code review. The point of this app is simple, the user opens it, taps "begin", they select their league - men's, women's or co-ed, and then they select their skill level.

The main issue I see with this is the fact that the button functions in the league VCs are all the same. I tried adding it to the BorderedButton.swift file, but it required that I add the player variable - which I figured was redundant to the fact that it is already in each league view controller.

Main View Controller:

class WelcomeVC: UIViewController 

 override func viewDidLoad() 
 super.viewDidLoad()


 

 @IBAction func unWindFromSkillVC(unwindSegue: UIStoryboardSegue) 

 

User arrives here after they select "continue" on the welcome screen. They then select which league they want to join. This gets the segues ready for the next view controller as well

League View Controller:

import UIKit

class LeagueVC: UIViewController 


 var player: Player!

 override func viewDidLoad() 
 super.viewDidLoad()

 player = Player()
 


 @IBAction func onMensTapped(_ sender: Any) 
 selectLeague(leagueType: "mens")
 performSegue(withIdentifier: "mensVCSegue", sender: self)

 

 @IBAction func onWomensTapped(_ sender: Any) 
 selectLeague(leagueType: "womens")
 performSegue(withIdentifier: "womensVCSegue", sender: self)
 

 @IBAction func onCoedTapped(_ sender: Any) 
 selectLeague(leagueType: "coed")
 performSegue(withIdentifier: "coedVCSegue", sender: self)
 


 func selectLeague(leagueType: String) 
 player.desiredLeague = leagueType

 

 override func prepare(for segue: UIStoryboardSegue, sender: Any?) 
 if let mensVC = segue.destination as? MensVC 
 mensVC.player = player
 else if let womensVC = segue.destination as? WomensVC 
 womensVC.player = player
 else if let coedVC = segue.destination as? CoedVC 
 coedVC.player = player
 
 

User arrives here after they select the "co-ed" league, then they select their skill level.

Co-ed View Controller (controls the co-ed view controller):

class CoedVC: UIViewController 

 var player: Player!


 override func viewDidLoad() 
 super.viewDidLoad()

 print(player.desiredLeague)
 

 override func didReceiveMemoryWarning() 
 super.didReceiveMemoryWarning()
 // Dispose of any resources that can be recreated.
 


 @IBAction func beginnerBtnPressed(_ sender: Any) 
 selectSkillLevel(skillLevel: "beginner")
 

 @IBAction func ballerBtnPressed(_ sender: Any) 
 selectSkillLevel(skillLevel: "baller")
 


 func selectSkillLevel(skillLevel: String) 
 player.selectedSkillLevel = skillLevel
 print(skillLevel)
 

User arrives here after they select the "women's" league, then they select their skill level.

Women's View Controller:

class WomensVC: UIViewController 

 var player: Player!

 override func viewDidLoad() 
 super.viewDidLoad()

 print(player.desiredLeague)
 

 override func didReceiveMemoryWarning() 
 super.didReceiveMemoryWarning()
 // Dispose of any resources that can be recreated.
 


 @IBAction func beginnerBtnPressed(_ sender: Any) 
 selectSkillLevel(skillLevel: "beginner")
 

 @IBAction func ballerBtnPressed(_ sender: Any) 
 selectSkillLevel(skillLevel: "baller")
 


 func selectSkillLevel(skillLevel: String) 
 player.selectedSkillLevel = skillLevel
 print(skillLevel)
 

User arrives here after they select the "men's" league, then they select their skill level.

Men's View Controller:

import UIKit

class MensVC: UIViewController 

 var player: Player!

 override func viewDidLoad() 
 super.viewDidLoad()

 print(player.desiredLeague)
 

 override func didReceiveMemoryWarning() 
 super.didReceiveMemoryWarning()
 // Dispose of any resources that can be recreated.
 


 @IBAction func beginnerBtnPressed(_ sender: Any) 
 selectSkillLevel(skillLevel: "beginner")
 

 @IBAction func ballerBtnPressed(_ sender: Any) 
 selectSkillLevel(skillLevel: "baller")
 

 func selectSkillLevel(skillLevel: String) 
 player.selectedSkillLevel = skillLevel
 print(skillLevel)
 

Here is the GitHub link: https://github.com/andrewlundy/app-swoosh

The first thing that I noticed is that you have three view controller
classes (MensVC, WomensVC, CoedVC) with identical code.

You can reduce that to a single view controller class (perhaps SkillsVC ?) which is the custom class for all three view controllers.

The prepare(for:segue:) method in LeagueVC then simplifies as well:

override func prepare(for segue: UIStoryboardSegue, sender: Any?) 
 let skillsVC = segue.destination as! SkillsVC
 skillsVC.player = player

This is also a valid use-case for a forced cast: If it fails then
we have a programming error, which should be detected early.

You could even replace the three view controllers by a single one in
the storyboard. The only difference between them is the background
image, and that can be loaded (e.g. in viewDidLoad) depending on
the chosen league.

With respect to the duplicated button action code: I find it acceptable
for three buttons. An alternative is to add outlet connections to
each button in the LeagueVC and connect all buttons to the same
action method:

@IBAction func leagueButtonTapped(_ sender: UIButton) 
 switch sender 
 case mensButton:
 selectLeague(leagueType: "mens")
 performSegue(withIdentifier: "mensVCSegue", sender: self)
 case womensButton:
 selectLeague(leagueType: "womens")
 performSegue(withIdentifier: "womensVCSegue", sender: self)
 case coedButton:
 selectLeague(leagueType: "coed")
 performSegue(withIdentifier: "coedVCSegue", sender: self)
 default:
 fatalError()
 

Another option would be to attach the segues directly to the buttons
and remove the button action methods. Then the logic has to be put
into prepare(for:segue:), depending on the segue's identifier.