octave-bug-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Octave-bug-tracker] [bug #35329] change of paperorientation does not up


From: Rik
Subject: [Octave-bug-tracker] [bug #35329] change of paperorientation does not update papersize
Date: Tue, 24 Jan 2012 17:18:44 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20100101 Firefox/5.0

Follow-up Comment #10, bug #35329 (project octave):

Good patch Ben.  I can see you're going to be a full-fledged C++ programmer
soon.  

I made some style changes and checked them in here
(http://hg.savannah.gnu.org/hgweb/octave/rev/08779abcb640).  I'll explain what
I did so you can see the method in my madness.

I like to carryover the Octave spacing convention for functions and indexing
that exists in m-files.  To whit, use a space between the function name and
the opening parenthesis, but cuddle the parenthesis when the name is a
variable and the operation is indexing.  Here is a good example from earlier
in graphics.cc.


static Matrix
default_data (void)
{
  Matrix retval (1, 2);

  retval(0) = 0;
  retval(1) = 1;

  return retval;
}


This is completely optional, but I like using the in-place operators such as
*=, +=, etc.  In m-file implementations these are 3X faster.  In C++, gcc is
probably smart enough to optimize away the temporary.  However, who wants to
take a chance?


  pos(0) = pos(0) * 72;
     =>
  pos(0) *= 72;


The graphics property framework has a front end and a back end.  The front end
(set routines) has to deal with caseless strings because people will type
anything in any format.  However, the values are always stored in a
standardized format and the back end (get routines) always return a lower case
version of the property in question.  I changed some of your patch to use
std::string where it was possible.  The equality operator is overloaded for
strings so this allows the code to be simplified.


  caseless_str porient = get_paperorientation ();
  if (porient.compare ("landscape"))

     =>

  if (get_paperorientation () == "landscape")


There's no way that you would know about it, but the swapping of temporary
values happens so frequently that C++ includes a routine for it in the std
library.  I rewrote the following.


  double tmp;
  tmp = a;
  a = b;
  b = tmp;

    =>

  std::swap (a, b);


Finally, I added the 'const' keyword to your declarition of papertype
conversion values.  The compiler might have figured out that you only assigned
to these once.  But, it is better to be explicit and tell the compiler that. 
In this case, it can then do all the multiplication and division only once at
compile time and the magic values are stored in the code.  Otherwise, there is
a chance that these values are computed again every time the function is
called.


  const double mm2in = 1.0 / 25.4;
  const double tol = 0.01;


This may look like a lot, but it really isn't.  These are style issues only
and the code works without them.


    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?35329>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]