I’m a bit confused with the activity on unit 4 lesson 11 activity 3; it seems to ask kids to use functions but I don’t see how they can use functions with this code because not much repeats. Here’s the code that I used:
var text;
var font;
var color;
var size;
onEvent(“quoteInput”, “input”, function( ) {
text = getText(“quoteInput”);
setText(“quoteOutput”, text);
});
onEvent(“fontFamilyInput”, “input”, function( ) {
font = getText(“fontFamilyInput”);
setProperty(“quoteOutput”, “font-family”, font);
});
onEvent(“colorInput”, “input”, function( ) {
color = getText(“colorInput”);
setProperty(“quoteOutput”, “background-color”, color);
setProperty(“colorOutput”, “background-color”, color);
});
onEvent(“fontSizeInput”, “input”, function( ) {
size = getText(“fontSizeInput”);
setProperty(“quoteOutput”, “font-size”, size);
});
What you are asked to do is create a single function that can be used in all of the onEvent callback functions. It is a bit of abstraction. Could you collect all of the update portions of the callback functions you have now and combine them into a single function called updateScreen?
I did and it works fine, but an error message still comes in the debug console. I believe it’s because the initial values are undefined. Here’s my modified code:
var text;
var font;
var color;
var size;
onEvent(“quoteInput”, “input”, function( ) {
text = getText(“quoteInput”);
updateScreen();
});
onEvent(“fontFamilyInput”, “input”, function( ) {
font = getText(“fontFamilyInput”);
updateScreen();
});
onEvent(“colorInput”, “input”, function( ) {
color = getText(“colorInput”);
updateScreen();
});
onEvent(“fontSizeInput”, “input”, function( ) {
size = getText(“fontSizeInput”);
updateScreen();
});
function updateScreen() {
setText(“quoteOutput”, text);
setProperty(“quoteOutput”, “font-family”, font);
setProperty(“quoteOutput”, “background-color”, color);
setProperty(“colorOutput”, “background-color”, color);
setProperty(“quoteOutput”, “font-size”, size);
}
And here’s the error message (it keeps popping up until I define everything; color, font, size).
WARNING: Line: 23: setProperty() value parameter value (undefined) is not a string.WARNING: Line: 24: setProperty() value parameter value (undefined) is not a string.WARNING: Line: 25: setProperty() value parameter value (undefined) is not a string.WARNING: Line: 26: setProperty() value parameter value (undefined) is not a number.
Suggestions on how to address? I think this might be confusing for the kids.
Greetings. I’m just curious why you chose the values you did…those are the same values I saw in the exemplar that Code.org provides, and interestingly, the same values a few of my students had.
When I wrote my version, I left the variables empty, and everything ended up working just fine…there was no difference between my app’s behavior and the app found on stage 2. Wouldn’t the point of doing something like var quote = “”; vs var quote; merely be to remind you as a coder of the type of data you plan to store is a string…more from a planning/point of reference perspective?
Also, why 24 for the fontSize? I just don’t see where that came from since the default size associated with the element’s properties is 20…and again, why set that from the beginning anyhow if there is a default size already? I can understand storing a value if that value didn’t already exist, but not in this context.
Thanks in advance for any insights you might be willing to share.
Initializing them to an appropriate type does tell us what we expect to be there. However, we need them to be initialized to something that at least doesn’t throw an error when we call updateScreen. So I use strings and numbers as appropriate. In this case I just used the example defaults.
Those values should represent the initial values set. In looking at I do see why you have a question.
quote is empty because we have not entered one yet. That seems good.
fontFamily can be empty it just uses what ever the default font is, which I believe is Arial. We could have set it to "Arial" and that would be better.
color is set to "lightgrey" because that is the color it starts as. An empty sting defaults to white.
Setting fontSize to 24 is an undiscovered bug. It should be 20.
Thank you so much for such a quick and helpful response.
I’m still a little baffled by two things.
If I change var color = “lightgrey” to var color = “lightgreen” (or any other color), I still see lightgrey, which I gather is because that’s the default background color specified in the properties (?):
However, why write var color = “lightgrey” (or any other color) if it doesn’t seem to change that setting?
Also, in looking a little more closely at the exemplar, I think I can see why removing the “” or not adding a fontSize = 24 would throw up errors… by checking for all of those setProperty conditions in the updateScreen function across all event handlers (every time the function is called) without the input that is specific to each of those event handlers individually, the errors are caused…is that right? When I remixed the exemplar and moved them out of the function and back into each respective event handler, starting the variables off as empty didn’t cause any issues at all
The reason I didn’t bundle all of the setProperty(s) into my function like the exemplar is I didn’t see how doing that made the code any more efficient. In fact, I thought that calling the function every time w/ all of those setProperty(s) bundled like that was more CPU calculation than was needed (less efficient?).
Here’s what I had:
So, I guess now my question is why would their approach in the exemplar still be the preferred way to go? I’m sure there must be a good answer…I just lack the experience.
Yes, in the case of the background color what you set in design mode is what shows up first. If you do change the initialization to “lightgreen” it changes color when you change the font first. That violates the Principle of Least Astonishment.
We use light grey because that is the color it starts at. Not changing color if you didn’t change the color drop down conforms to the Principle of Least Astonishment.
Yes, sending the value undefined as a parameter causes an error. undefined is an actual value that you can set a variable to and variables start with that value. For me, it doesn’t run correctly without the initialization. The original poster had that problem as well. Your version doesn’t have the updateScreen function.
The lesson being taught is about abstraction, simplification, and readability of code. Creating a single function that updates the entire screen makes all three of those good qualities happen.
Prior to around the 1980s the computer was more expensive than people. As such, we did everything we could to spare CPU cycles. The result was code that people could not understand easily. Late 1970s and by 1980 software people realized that people were more expensive than computers. That caused a revolution in the way we write code. Cost is driven by people reading other people’s code. Since then there are very few applications where that level of optimization over readability is considered a good idea.
The exemplar approach is a better way to go in terms of teaching our students the current thinking in software development.
Great points – thanks so much for taking the time to elaborate! I’m excited to learn about the Principle of Least Astonishment…I hadn’t heard of that before, but I looked it up, and that seems like a great rationale for why we take the conventional approaches we do (mental models). I also appreciate the comment, “The lesson being taught is about abstraction, simplification, and readability of code. Creating a single function that updates the entire screen makes all three of those good qualities happen.”
One lingering question: Other than not containing the setProperty(s) of the exemplar’s updateScreen function, isn’t my checker function serving the same purpose (i.e., updating the screen)? My app doesn’t seem to behave any differently from the exemplar app. I thought one of the main ideas of a function was to remove repeated code (Unit 4, Lesson 10):
In my way of thinking, the if statements in my check function are what would have repeated in every single event handler, which is why they were placed into a function. However, I didn’t include the setProperty(s) because each of those are unique. Are you saying that it’s the best practice of all to place as much code into the function as possible, even if it isn’t exactly repeating? I can see where that might play a role with readability, but I guess I haven’t developed a sense yet for that because both seem equally as readable to me. However, I want to know if that’s how I show be seeing that so I know how to better gauge this in the future.
In a program this small nobody notices. It is about the lesson. It is true that checker does reduce redundant code. But it isn’t just about redundant code. Redundant code is a bug waiting to happen. Spreading the code that touches the GUI all over the place is also a bug waiting to happen. Now, in this example, there isn’t that much code to spread all over the place. The idea is to get our students looking at good code from the beginning so they develop good habits.
Functions are also used instead of comments to make a program more readable. It is not unusual to see a function with one line of code that is used one time. Why? To add an explanation of what is happening without a comment. I name the function based on what that single line of code does.
One thing that creating an updateScreen function does is put everything that sets properties of the GUI in one place. I would write something like this:
Putting everything that gets properties in one place as well. Then I created a third function that combines them and is used in all the onEvent calls. I also created a function to judge the combinations.
When we look at the function updateScreen now it reads like a comment; get screen properties, judge the combination, then set screen properties. The individual functions are simple. They have 4 getProperties or setProperties calls in a row with no logic. The logic is cloistered into a separate function called judgeCombination. When we look at each function it is small with only one clear goal described by the name.
Your feedback has been tremendously helpful. I definitely feel much better around my understanding of functions and their use. I hope you don’t mind, but I’m going to save some of your comments to share with my students. The same is true of the sample Make you coded (along w/ your explanation). I didn’t realize how versatile functions could be – even with such a small app. Thanks so much for taking the time to write that as an example. You mentioned the importance of abstraction, simplification, and readability, and it’s pretty obvious that your code nails all three.
Thank you for sharing your example. I find it much more clear than the exemplar in terms of how it is organized. I came across this conversation looking for the same information as Mr. Legatt - my students were getting the error since they weren’t initializing their variables. While looking at your example, I am curious why your version of this program doesn’t get an error - you didn’t give the variables an initial value? I guess I am still a little confused why it is necessary? Thanks for any added insight you can give.
getScreenProperties is always called before setScreenProperties. So there is no need to initialize. If you see what I am saying. You always assign the variables a value before you use the variables. Initialization is what you do if there is a possibility of using the variable before you assign it a good value.
It may be helpful not to think of it as some arbitrary rule that says variables must be initialized.
To my understanding, the problem is trying to use/read a variable that doesn’t have a value. As long as the variable is assigned a value sometime before it is used/read, should be okay. This can be done by initializing the variable when it’s declared, or assigning it a value somewhere down the line before it is used/read.
