Here I am, back again, much sooner than expected, as I have discovered a previously unknown problem, one that I hadn’t tested for because it hadn’t occurred to me it would be a problem, or that the condition even existed, but it turns out I was wrong.
The issue has to do with one of null values. Player information that I download from the source site has an entry for what school a player attended. This seems like an easy thing to deal with as the rules require every player be one year removed from high school and thus you hear about the ‘one and done’ rule. However, this does get complicated when you’re dealing with players from every country. The rules are slightly different in regards to them, and it turns out that the NBA web site doesn’t treat all players equally.
Basically, some players do have the schools entry filled in even though they didn’t go to a US College. For instance, the raw data for Pau Gasol lists “Barcelona, Spain” in his school field, and when uploading his information the first time he’s found, the upload works perfectly. However, not all foreign born players have information listed in school.
I don’t know why or how the thought process goes, but some European players have nothing listed in the school entry, and when you download the information it lists null in the index where the school information should be. It turns out in my thought processes in developing this application, it had never occurred to me that the school information would not be provided. As such, I made it a required field within my database design, and when I tested my code for getting the player, I chose a single player file that had all the information filled in properly. If I had chosen a player with a null school, or tweaked the existing file to have an alternative where the school was null, I would have caught this iss7ue.
The simple thing is of course to change the way that the school is handled, and give it a default value if it is blank. However, the test has to be written first to verify that this will work.
Using the sample file I used for my previous player testing, I created an alternate JSON file where the school entry was nil (as returned by the method in my application). I wrote a test that would check that the record count would change by 1 upon processing this new JSON file and that the school entry would have a value of ‘N/A’, which I planned to set up as a default.
After resetting my test database to dump all the information put in there before (note to self, check RSpec settings to empty test database after all tests where I load the data), I ran the test on the Player model functionality which now fails twice. To get it to pass, I just need to set a default value for the school entry to ‘N/A’. I’m going to do this through a migration.
A quick search of the Rails Guide on migrations finds a method built-in to do exactly what I want;
change_column_default. A quick rails migration should to the trick:
change_column_default :players, :school, "N/A"
Due to the note regarding the reversibility I wrote this as an up/down where the down changed the default back to nil. This is for completeness, it’s doubtful it would ever be reversed
And voila, failure. This was not a simple fix. Reading the errors closely it turns out that JSON doesn’t like when it comes upon a nil entry and give you an error
399: unexpected token at error. Thus, we go to back to the internet to find how to deal with it.
A bit of research finds the JSON rules that I’ve never actually seen before. Down there in section discussing the acceptable values, it lists nil but not null. I had used the nil value in my JSON test file, so I converted it to null and now the test still failed, but as previously, so changing the default value didn’t work, so more research to do.
A quick jump into the rails console shows me that while
Player.new will create the “N/A” value as default for the school, my processing system over rides that as it sets a value for school looping through the array of player information which gets set to nil while processing. So, the default method was not a good idea since my code was set to over ride any default values, proper or not. This means of course, that I have to go into the method itself to fix the problem, which hopefully won’t be too difficult.
A simple if/else statement solves the issue. JSON parses the null to nil in ruby, and everything in ruby is an object so I can just use the
.nil? method on that member of the array. Set the school to “N/A” if you get nil or to the school value in the entry. This solution, while probably not the most eloquent (I could have used a ternary, but I’m not great at those, maybe when I refactor) does get the test to pass though, and that’s always the goal.
I repopulated my ‘first day’ information and looked for ‘nil’ entries within my statistics, and they still exist, but only two of them, so now we examine those two and see what I missed. If I had to guess, I missed another null entry that I hadn’t thought of before. My first blank was a player whose name is Rondae Hollis-Jefferson. For some reason, the NBA doesn’t seem to thing Mr Hollis-Jefferson has a country affiliation because that is listed as null within the JSON, Obviously this is a glitch in the system, but one I have to account for so I get the relevant information I need. It turns out the second bad information, for player Sasha Kaun is also related to a null country entry.
So, two players with null country entries, weird, but I guess this is what is meant by and edge case. I will do this one slightly differently due to the results of the last one. No migration this time, just a failing test and the if statement (which I will write as a ternary this time) to set the country to a default value in case of no entries.
Ternaries (for those who don’t know) are a special way to write a simple if/else statement. They’re cleaner and easier to read for coders when the test (and results) are pretty simple. It works something like this:
test_condition ? true_result : false result
My understanding is that the question mark indicates that the code to the left is a test that should yield either a true or a false outcome, and then to the right of the question mark, you get what should yield if the condition is true, the colon represents the else, and then the false result.
In this case, my ternary looks like:
player_info.nil? ? country = "Unknown" : country = player_info
Dropping this in to my failing ‘null country’ test made it past, so I then rewrote the earlier if/else for null school value as a ternary and ran the test again. It all cleaned up and passed.
So, did I handle all the null conditions that might come up? I’m not 100% sure at this time. I wouldn’t really have thought of an null country affiliation for a player. I mean everyone comes from somewhere right? I should probably take into consideration all null possibilities going forwards (what happens if a one name player enters the league, like his legal name is one name, how will the league handle that?), and I will, but I am anxious to actually start writing queries, and testing them.
Now then, since I had missed this issue before, my ‘successful’ test in part 2 of this series, isn’t exactly successful anymore, because the total number of Players will change. Thus I have to reset all my dump files and tests once i verify ‘no nils’ on my downloads.
Three days and 29 games downloaded and there were no entries of player statistics not associated with a player, which I’m taking as a good sign to move forward, so now I will reset my dump files, and then I’ll be back to where I was at the end of part 2 of this process.
However, I think it’s a good lesson. Just because you think everything is running smoothly, it might not be. Don’t freak out. Isolate the problem, investigate it, and fix it. Thankfully, the fix here was simple, so my ‘step back’ only put me back a few hours (whereas in the past I might have let emotion take over and it might have been days before I looked at it). Positive steps are positive, right?