I'm revisiting CS50 and have been trying to strengthen my programming skills. I recently tackled a Scrabble scoring project, and while I had some fun with it, I'm looking for feedback on my code. I've got two functions for calculating scores for each player and a function to determine the winner. I've put in about an hour and a half into it, but I'm aware there might be better approaches. I'd really appreciate any critiques, especially regarding potential redundancies and any critical issues. Thanks!
2 Answers
Great job for a beginner! You definitely want to apply the DRY principle — your score functions are a perfect case for that. If you find yourself repeating code, just pull that into a separate function. Also, keep an eye on your usage of global variables; they can cause issues in larger projects. It's better to keep them local where possible, especially your score variables.
Consistent naming conventions will also help your readability. For example, you're using different styles like `Score1` and `alphabetSize`, so pick one and stick to it. Lastly, you could simplify your `whoWins` function by just having `return "Tie";` without the else statement. Plus, on comments, aim for them to clarify the 'why' rather than the 'what'; this keeps your code cleaner!
Your code is solid for someone just starting out! A couple of suggestions: instead of having separate `calcScore1` and `calcScore2` functions, you could combine them into one function that takes a player as a parameter. That way, you avoid code duplication.
Also, when you're looping to calculate scores, rather than checking each letter against the whole alphabet, consider using ASCII values. For example, you could compute the index directly from the character's ASCII value like this: `'b' - 'a'` gives you the right index. Just remember to validate the letters first!
It's also best to keep your score variables local instead of global, and use `const` for your scores array since it won't change. And just a heads-up about memory management: C doesn't clean up automatically like some other languages do, so learning to free up memory will be crucial as your projects get bigger!
Thanks, that's really helpful! I realized the duplicate function situation and thought combining them would be good, just wasn't sure how to do it effectively. Also, I like the idea of using ASCII for efficiency. Regarding memory management, I've still got a bit to learn, but I appreciate the heads-up there!
Thanks for the awesome feedback! I recognize that I need to work on my consistency and cleanliness in naming, and I appreciate the tips on simplifying my functions. I'll work on cutting down on comments where unnecessary too!