Notes on homework 2

• Comments
  • Homework 3 due date has been pushed back.
  • Please see the ACM Code of Ethics, especially:
    • 2.1: Strive to achieve the highest quality, effectiveness and dignity in both the process and products of professional work.
    • 2.2 Acquire and maintain professional competence.
    • 2.4 Accept and provide appropriate professional review.
    • 2.5 Give comprehensive and thorough evaluations of computer systems and their impacts, including analysis of possible risks.
  • By the way, our standards are somewhat trivial
    • See Google C++ Style Guide.
  • At some point, when you have lost too many points, I stop grading.
  • If at any time you are submitting something as a question and not as a submission, please put QUESTION in the subject line.
  • If you would like to test your code against my test cases
    • copy ~dbennett/230/hw2/doit to your directory

    • cp ~dbennett/230/hw2/doit .
      

    • execute doit progname where progname is the name of your executable.

    • sh doit ./palindrome
      	     

  • No executables in an email message. If you don't understand this, ask RIGHT NOW.
  • Remind me to show you gdb
  • In case you are not sure, you CAN pass a file stream to a function, just pass it by reference.

    • void OpenFile(ifstream & infile){
      	    

  • I prefer to list the parameter identifiers in function prototypes

    • void Foo(string dogName, int dogAge, float dogCost);
      void Foo(string,int, float);
      	   

  • From now on, all elements of the standard template library are off limits.
    • This includes the standard container library.
    • And the standard algorithm library.
    • Code employing these functions will be given a 0 with no redo allowed.
• Major problems.
  • I know the compiler flags are a pain, get stuff cast the right way.
    • So the compiler is complaining about size_t
    • Don't just cast stuff at random

    • string s;
      cout << s.size()-1 << endl;
      	   

    • If you cast things the WRONG WAY you WILL HAVE PROBLEMS
  • Your code must work on the given test case.
  • Repeated code
    • I said I was picky about this. Don't repeat code.

    • same = true;
      for (i=0;i<phrase.size();i++) {
          if (phrase[i] != reverse[i]) {
             same = false;
          }
      }
      
      strippedSame = true;
      for (i=0;i<strippedPhrase.size();i++) {
          if (strippedPhrase[i] != strippedReverse[i]) {
             strippedSame = false;
          }
      }
      	    

  • Be careful of creating data files on windows.
  • Don't google and use stuff we have not discussed. Just don't do it.
    • What does string word[10] do? Probably not what you think.
  • Functions can have local variables.
    • If you use a parameter only in a function
    • And do not need it to pass information in or out
    • It should be a variable, not a parameter

      void Foo(int a, int b, int c, int & out) {
          a = 7;
          b = 5;
          c = 12;
          out = a+b/c;
      
          return;
      }
      	    

  • Make sure if you are indexing into a string with [i] or .at(i) make sure that 0 ≤ i < string.size()
  • Use my compile flags and NO WARNINGS
• Problems
  • Use if-else statements not two parallel if statements

    • // this is bad and can lead to errors
      if (a == b) {
         DoSameFunction();
      }
      if (a != b) {
         DoDifferentFunction();
      }
      
      // prefer this
      if (a == b) {
         DoSameFunction();
      } else {
         DoDifferentFunction();
      }
      	   

  • What do you expect a function called Compare to do?
    • Name things appropriately or documentation standards will go way up.
    • It should take two strings and return a bool
    • It should not do any reversals, stripping, ...
    • OR OUTPUT
  • If you are building a control structure, add code blocks.
    • Too many problems result from changing code without this

    • // this is error prone
      if (condition) 
         statement
      
      // this is better
      if (condition) {
         statement
      }
      	    

    • Same with ALL other control structures.
  • Your main routine should read like a high level algorithm.
    • No detailed code.
    • Not very many variables (two in this case, possibly 3)
    • File opening should be a function.
    • But don't do this either

    • int main() {
          DoMain();
          return 0;
      }
      	     

  • Make sure you spell and format output correctly, if not, you will really mess up my grading scripts.
    • And you need to follow all of the instructions, not just some of them.
    • Throwing away points on this is silly.
  • Don't change the LCV in a for loop outside of the loop statement. Readers don't expect this change. Use a while loop.

    • bool CompareStrings(string s1, string s2) {
          // assume both strings are the same.
          bool same = true;
          size_t i;
      
          if (s1.size() != s2.size() ) {
              same = false;
          }
      
          if (same) {
              for(i=0; i < s1.size(); i++) {
                  if (s1[i] != s2[i]) {
      	       same = false;
      	       // this is confusing code.
      	       i = s1.size()
          	    }
              }
          }
          return same;
      }
      
      // I think this is better code
      bool CompareStrings(string s1, string s2) {
          // assume both strings are the same.
          bool same = true;
          size_t i;
      
          if (s1.size() != s2.size() ) {
              same = false;
          }
      
          for(i=0;  same and i < s1.size() ; i++) {
               if (s1[i] != s2[i]) {
                   same = false;
          	 }
          }
          return same;
      }
      	

    • Some may argue that you should use a while loop not a for in this case. But either code example will violate that standard.
    • I feel changing the LCV outside of the for is much more likely to lead to errors.
  • Only one return per function and it should be the last line in the function.
    • This is especially a problem if you are using it to exit early from a loop.
• Minor problems
  • A local convention is that all variables are declared at the top of the function code block. Follow this.
  • Don't do work unless you need it.
    • Don't strip punctuation from a perfect palindrome.
    • Don't strip non-alpha from a natural palindrome.
    • Don't reverse anything you don't need to.

      void Foo(string phrase) {
          string reverse = Reverse(phrase);
          string stripped = Strip(phrase);
          string rstripped = Reverse(stripped);
          ...
      
          if (phrase == reverse) {
          ...
      }
      	    

  • Delete junk functions you have written but are not using, especially if they contain bad code.
  • What are the conventions for naming functions, variables, constants?
  • Don't make silly one liners without reason
    • This is tough.
    • I would consider a function for this

      • cout < '"' << phrase << "\" is a " << palClass << endl;
        	     

      • It is fairly complex
      • And possibly open to change.
      • A function call will be more clear than the code

      • PrintResults(phrase, palType);
        		 

    • I would not make a function for this

      • getline(inFile, phrase):
        		  

      • It is not complex
      • Or likely to change.
      • And the function call is no clearer than the code.
• Trivial problems.
  • Almost Always use an endl after output.
    • You need a reason not to.
    • This flushes the buffer.
  • All functions should have a return at the end.
    • Local convention. Not necessary, but seems to be a good practice.
• A Caution