Reputation: 5
I’m trying to use the if
statement below (I commented it out). The code works perfectly except that when I “uncomment” it, I get an error. Please help me with this issue and if you have any advice on things you’d change, please let me know as I’d rather not make a bad habit of things that should be done differently.
On that note, if you were born in the 1900’s and would like to try this code, please feel free! It’ll tell you the day of the week you were born on. Unless you were born in January or February (that’s why I need the if
statement fixed). Also, if you know where and when to add enums for the arguments called in the function (the day, month and year), that would be awesome as well.
class Birthday {
func findDayOfWeek(_ month:Int, _ day:Int, _ year:Int) -> String{
var runningNumber = 0
//print( "start with " , runningNumber)
let shortenedYear = (year - 1900)
let remainder = shortenedYear % 12
runningNumber += remainder
//print("add the remainder" , runningNumber)
var testNumber = runningNumber
while testNumber > 3 {
runningNumber += 1
testNumber -= 4
}
//print("because we added one for every four" ,runningNumber)
let dividedByTwelve = ((shortenedYear - remainder) / 12)
runningNumber += dividedByTwelve
//print("add the amount of 12’s to get to your year" , runningNumber)
var testNumber2 = runningNumber
while testNumber2 > 7 {
runningNumber -= 7
testNumber2 -= 7
}
//print("if the previous number was over 7, we looped down by 7 to get " , runningNumber)
**//if remainder == 0{
let doomsdayPerMonth: [Int:Int] = [ 1:31, 2:28, 3:7, 4:4, 5:9, 6:6, 7:11, 8:8, 9:5, 10:10, 11:7, 12:12]
//}
//else{
//let doomsdayPerMonth: [Int:Int] = [ 1:25, 2:29, 3:7, 4:4, 5:9, 6:6, 7:11, 8:8, 9:5, 10:10, 11:7, 12:12]
//}**
let doomsdayOfMonth = doomsdayPerMonth[month]
var daysUntilBDay = day - doomsdayOfMonth!
while daysUntilBDay < 0 {
daysUntilBDay += 7
}
runningNumber += daysUntilBDay % 7
//print("since the day of the month is \(day), looping down by seven would be \(daysUntilBDay % 7). Add that to above to get \(runningNumber).")
runningNumber = runningNumber % 7
//print(runningNumber)
let days: [Int:String] = [0:"Wednesday", 1:"Thursday", 2:"Friday", 3:"Saturday", 4:"Sunday", 5:"Monday", 6:"Tuesday"]
let months: [Int:String] = [1: "January", 2: "February", 3: "March", 4: "April", 5: "May", 6: "june", 7: "July", 8: "August", 9: "September", 10: "October", 11: "November", 12: "December"]
let dayPicker = days[runningNumber]
return ("\(months[month]!) \(day), \(year) was a \(dayPicker!).")
}
func writeInWords(_ month:Int, _ day:Int, _ year:Int) -> String {
let month1: [Int:String] = [1: "January", 2: "February", 3: "March", 4: "April", 5: "May", 6: "June", 7: "july", 8: "August", 9: "September", 10: "October", 11: "November", 12: "December"]
let monthName = month1[month]
return ("\(monthName!) \(day), \(year)")
}
}
let personBday = Birthday()
print (personBday.findDayOfWeek(3,24,1914))
//print(personBday.writeInWords(4,27,1988))
Upvotes: 0
Views: 1508
Reputation: 3699
Since you asked for advices here are my two cents: First, the class Birthday might contain the variables for a specific birthday, such as
class Birthday {
let month: Int
let day: Int
let year: Int
}
This way the instance method would be with no parameters. The method you wrote is self sufficient and do not need an instance to work, so if you want to keep it this way I would make it static as it can compute any birthday day.
class Birthday {
func findDayOfWeek(_ month:Int, _ day:Int, _ year:Int) -> String{
Code style is quite important for many dev teams. I would suggest you to follow this code style guide. With these indications your method would become
func findDayOfWeek(forMonth month: Int, day: Int, ofTheYear year: Int) -> String {
What we did here is to give a clear indication of what the parameters represents when the method invocation is performed. it would be otherwise very confusing after a couple of months to see an invocation like finDayOfWeek(7, 24, 1986)
especially who didn't write the method will find not intuitive which numbers represents what. findDatyOfWeek(forMonth: 7, day: 24, ofTheYear: 1986)
is much more clearer.
var runningNumber = 0
//print( "start with " , runningNumber)
let shortenedYear = (year - 1900)
let remainder = shortenedYear % 12
runningNumber += remainder
Try to initialise variables as close as possible to their usage. In this case instead of initialising runningNumber
to zero at the beginning of the method you could have initialised directly with the value of remainder in this last line.
//print("add the remainder" , runningNumber)
var testNumber = runningNumber
while testNumber > 3 {
runningNumber += 1
testNumber -= 4
}
//print("because we added one for every four" ,runningNumber)
let dividedByTwelve = ((shortenedYear - remainder) / 12)
runningNumber += dividedByTwelve
//print("add the amount of 12’s to get to your year" , runningNumber)
var testNumber2 = runningNumber
testNumber finished its purpose earlier in the function. You could reuse it here.
while testNumber2 > 7 {
runningNumber -= 7
testNumber2 -= 7
}
//print("if the previous number was over 7, we looped down by 7 to get " , runningNumber)
**//if remainder == 0{
let doomsdayPerMonth: [Int:Int] = [ 1:31, 2:28, 3:7, 4:4, 5:9, 6:6, 7:11, 8:8, 9:5, 10:10, 11:7, 12:12]
//}
//else{
//let doomsdayPerMonth: [Int:Int] = [ 1:25, 2:29, 3:7, 4:4, 5:9, 6:6, 7:11, 8:8, 9:5, 10:10, 11:7, 12:12]
//}**
Just like others said, here the problem was the declaration of a local variable within a scope. But again, code style and code readability is important. So careful with spaces and dictionaries with literals. Also, the two dictionaries are exactly the same except for the first two items. I would refactor this code with:
var doomsdayPerMonth = [1: 31, 2: 28, 3: 7, 4: 4, 5: 9, 6: 6, 7: 11, 8: 8, 9: 5, 10: 10, 11: 7, 12: 12]
if remainder != 0 {
doomsdayPerMonth[1] = 25
doomsdayPerMonth[1] = 29
}
Also, since the keys are Integers I would have used an array and not dictionary, so the whole thing would look like that:
var doomsdayPerMonth = [31, 28, 7, 4, 9, 6, 11, 8, 5, 10, 7, 12]
if remainder != 0 {
doomsdayPerMonth[1] = 25
doomsdayPerMonth[1] = 29
}
The rest of the code can be refactored accordingly to the previous suggestions.
Upvotes: 0
Reputation: 318814
Your commented code needs to be written as:
let doomsdayPerMonth: [Int:Int]
if remainder == 0 {
doomsdayPerMonth = [ 1:31, 2:28, 3:7, 4:4, 5:9, 6:6, 7:11, 8:8, 9:5, 10:10, 11:7, 12:12]
} else {
doomsdayPerMonth = [ 1:25, 2:29, 3:7, 4:4, 5:9, 6:6, 7:11, 8:8, 9:5, 10:10, 11:7, 12:12]
}
The problem with your version is that you declare local variables that have no scope outside of the if/else
statement.
Please not that your entire weekday finding method can be written as:
func findDayOfWeek(_ month: Int, _ day: Int, _ year: Int) -> String {
let date = Calendar.current.date(from: DateComponents(year: year, month: month, day: day))!
let weekday = Calendar.current.component(.weekday, from: date)
let formatter = DateFormatter()
return formatter.weekdaySymbols[weekday - 1]
}
And here's your whole class with some rework. No need to hardcode month and weekday names.
class Birthday {
let formatter: DateFormatter = {
let fmt = DateFormatter()
fmt.dateStyle = .long
fmt.timeStyle = .none
return fmt
}()
func dayOfWeek(_ month: Int, _ day: Int, _ year: Int) -> String {
let date = Calendar.current.date(from: DateComponents(year: year, month: month, day: day))!
let weekday = Calendar.current.component(.weekday, from: date)
return formatter.weekdaySymbols[weekday - 1]
}
func writeInWords(_ month: Int, _ day: Int, _ year: Int) -> String {
let date = Calendar.current.date(from: DateComponents(year: year, month: month, day: day))!
return formatter.string(from: date)
}
}
let bd = Birthday()
print(bd.dayOfWeek(3, 24, 1914))
print(bd.writeInWords(1, 27, 2018))
Output:
Tuesday
January 27, 2018
Upvotes: 3