Menu
▾
▴
Re: [Tsheetx-developers] if else bracketing
|
From: Mark W. <ma...@rw...> - 2011-04-19 12:25:07
|
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
Dear All, <br>
<br>
Since I started on the tsng project several months ago my aims have
been to:<br>
<br>
1) Convert the project to OO<br>
2) Improve the code structure<br>
3) Remove some of the code that involves a degree magic through the
use of includes and passing variables between these includes moving
to a class based structure<br>
4) Improve the error handling, error reporting, and debugging
capabilities<br>
5) Create a new framework that aids devleopment and makes the
project maintainable.<br>
<br>
To review:<br>
1) Much of the code is now functioning in the OO framework but quite
a lot of work is still required. Once the code 'works' as it once
did, we can move onto stage 2: restructuring the code that handles
the db queries and generating the tables / content. As it currently
stands I haven't really done anything to the actual application
level code.<br>
<br>
2) This is coming on slowly. The templating system has reduced the
quantity of code in each of the page files. When everything 'works'
again, stage 2 can commence which will make everything more readable
and understandable.<br>
<br>
3) Many pages included a file i.e. the navcal stuff that required
certain variables are set in the parent file so that navcal receives
the correct information. This is very bad programming practice as
there is no traceability and things either work or they don't. It
takes a lot of work to realise the coupling between the two files.
Therefore navcal is now class based and the required variables are
passed in through constructors and/or function calls. Readability
& Maintainability = 1 vs Magic = 0.<br>
<br>
4) The error handling whilst still in its infancy, has the
capability to speed up looking for errors in code. i.e. the error
screens display snippets of source code. Great for debugging. Also
the Debug class flags can be extremely useful. A good example is if
you click submit that then calls a *_action page, if you enable
Debug::$location then the _action page won't automatically redirect
back to where you originally came from allowing you to see any debug
statements related to that particular submission.<br>
<br>
5) Lots of good work is currently going on i.e.
Internationalisation, new Config, new Daily etc and all of this is
using the new framework i've spent quite a long time working on.
The other thing that is lacking in the project is comments.
Comments allow other developers to gauge an understanding before
reading code. It's not a perfect, example but look at
config.factory.class.php as it has quite a few comments before
functions, and inline. In comparison, just look at how poor the
commenting in common.class.php is? Using Javadoc style commenting
before functions and classes allows IDE's like eclipse to give you
snippets of documentation when you need it most. The eclipse auto
completion tools and javadoc popups are extremely useful when you
need to establish what variables need passing to a function.<br>
<br>
<br>
The common theme in everything I have done so far is to improve the
structure, readability and maintainability of the code in the
project. As I work on completing #1 I am changing a large number of
files simultaneously - see my last commit that updated 72 files! <br>
<br>
I have to say that I am slightly bemused as to why Scott has even
brought the if else problem up. I can't say whether I have or
haven't changed the structure of an already existing if else
statement to leave a carriage return between the } and the else.
But I more than likely have as I am working on changing so many
files. I will watch out next time i'm doing any work on tsng as I
can't say I intentionally change the ifelse structure. Many of the
changes I make in the OO change involve a degree of find/replace and
copy/paste.<br>
<br>
In my opinion this is more tidy and readable:<br>
<br>
if($i==1){<br>
echo 'i is one';<br>
echo '<br />';<br>
}<br>
else if($i==2){<br>
echo 'i is two';<br>
echo '<br />';<br>
}<br>
//inline comment<br>
else{<br>
echo 'i is not one or two';<br>
echo '<br />';<br>
}<br>
<br>
The alternative:<br>
<br>
if($i==1){<br>
echo 'i is one';<br>
echo '<br />';<br>
}else if($i==2){<br>
echo 'i is two';<br>
echo '<br />';<br>
}else{<br>
echo 'i is not one or two';<br>
echo '<br />';<br>
}<br>
<br>
The ifs and elses are all aligned with each other so when you are
glancing through code quickly the different cases can be seen very
quickly and easily. By having just a } on a line gives an amount of
separation that allows you to see the next if case. This format
also allows inline comments before an elseif/else statement.
Looking at Scotts preferred cuddled method, the next elseif case is
disguised in the rest of the code as there are no whitespace gaps
between the end of the first if case and the start of the second
making it more difficult to read. This is exacerbated when you have
large amounts of code in your if statement and / or long lines of
code at the beginning or end of each if case.<br>
<br>
In another example would you ever do this?:<br>
<br>
function($foo){<br>
echo "variable: ".$foo;<br>
}function($bar){<br>
echo "variable: ".$bar;<br>
}<br>
<br>
The answer is probably no. You would expect:<br>
<br>
<br>
function($foo){<br>
echo "variable: ".$foo;<br>
}<br>
<br>
function($bar){<br>
echo "variable: ".$bar;<br>
}<br>
<br>
What we should actually have is:<br>
<br>
/**<br>
* this is a description<br>
* @params String $foo - a random string<br>
*/<br>
function($foo){<br>
echo "variable: ".$foo;<br>
}<br>
<br>
/**<br>
* this is another description<br>
* @params String $bar - another random string<br>
*/<br>
function($bar){<br>
echo "variable: ".$bar;<br>
}<br>
<br>
Going back to the if elseif scenario, why bother to cramp the code
up making it more difficult to scan through? I've always followed
the coding standards rule of "do one thing and only one thing on
each line". Therefore I end an if case on one line and start the
elseif statement on the next line.<br>
<br>
<blockquote>Quote:18/04/2011 18:09GMT from Scott<br>
I do think that, in general, when working on an open source
project, new people should attempt to abide by whatever existing
style already exists, so long as that style is relatively sensible
and consistent.<br>
</blockquote>
I don't particularly buy into the statement above. That is akin to
saying I was here before you so therefore I am right. I am guessing
by the fact that we now have 4+ people working in the 2.0 branch
that the changes that I have been working on have been accepted by
the project. I therefore feel that the greater meaning to your
comment is slightly belittling. Before I started there wasn't a
huge amount of development being done. Now it appears that my work
has spurred more people to get back into development and the commit
rate of the project is significantly higher. TSNG is going through
a style change from functional PHP programming to OO PHP Programming
and with that comes a new style of code structure etc. Peter
acknowledged (16/04/2011 08:09GMT) that my preferred if else layout
improves the readability of the code, and that is what i'm trying to
achieve - more readability and better structure.<br>
<br>
<blockquote>Quote:14/04/2011 15:57GMT from Scott<br>
I realize change is sometimes disconcerting, so, play with it as
we develop it, and I think you'll see you're perhaps just trying
to cling to something that is familiar to you now.<br>
</blockquote>
Change as you rightly said Scott, is something that should be
embraced. Improvements can only be made if things change and I
think you are clinging onto an old style of practice for the sake of
change. White space is a good thing!<br>
<br>
<blockquote>Quote:18/04/2011 10:10 from David<br>
<pre>At some point early in the project I did apply a style to tidy up contributions from various people,
but as new people an contributions come then things change
(Entropy requires no maintenance. -- Markoff Chaney).</pre>
<pre>The best rule of thumb is:
keep functional changes separate from formatting/style changes. So Scott, put your pet peeves away at
the moment while the new code is written. I have seen projects where developers religiously
reformatted to their personal style on every commit, and you "can't see the wood for the trees".</pre>
</blockquote>
David does make a good point and i would like to add that i'm not
just reformatting code, i'm making changes to make tsng actually
work in the new framework.<br>
<br>
Regards<br>
Mark
<pre class="moz-signature" cols="72">_____________________________________________
Mob: 07725 695178
Email: <a class="moz-txt-link-abbreviated" href="mailto:ma...@rw...">ma...@rw...</a></pre>
<br>
On 18/04/2011 18:09, Scott Miller wrote:
<blockquote
cite="mid:BANLkTin=sbN...@ma..."
type="cite">Breaking your hand, that sucks, wishing you a speedy
recovery.<br>
<br>
Yeah, I see the point about Mantis.<br>
<br>
Codestyle: I'm not upset about the "cuddled / uncuddled elses",
but my first instinct is to go fix them all, and I realize that'
not what I should be spending my time doing. So I was hoping that
if Mark was amicable toward keeping elses cuddled, he would simply
stop uncuddling the elses. I do think that, in general, when
working on an open source project, new people should attempt to
abide by whatever existing style already exists, so long as that
style is relatively sensible and consistent.<br>
<br>
-Scott<br>
<br>
<div class="gmail_quote">On Mon, Apr 18, 2011 at 9:10 AM, David
Thompson <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:tom...@us...">tom...@us...</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt
0.8ex; border-left: 1px solid rgb(204, 204, 204);
padding-left: 1ex;"> Hi people.<br>
<br>
I have been quiet recently mostly due to work, but I also had
the misfortune a week ago to break my hand. But I have been
following what's been going on.<br>
<br>
So firstly, I want to say that I think it's great what
momentum and progress you have achieved on 2.0 so far. In the
past we have never had much more than two people on anything
at any one time. Thanks to everyone! I just wish I had more
time to help.<br>
<br>
I want to respond on a couple of points.<br>
<br>
Regarding Mantis: I agree with Mark - use it to record long
term TODOs, for immediate work on getting 2.0 stable and fully
featured the mailing list is best. But this is a good
reminder, try to prioritise and push things that can wait onto
Mantis, that way we will get 2.0 out quicker.<br>
<br>
Regarding codestyle: Somehow I'm honoured that you would
consider that I should decide, but you know that you can't win
on that one. At some point early in the project I did apply a
style to tidy up contributions from various people, but as new
people an contributions come then things change (Entropy
requires no maintenance. -- Markoff Chaney).<br>
<br>
The best rule of thumb is: keep functional changes separate
from formatting/style changes. So Scott, put your pet peeves
away at the moment while the new code is written. I have seen
projects where developers religiously reformatted to their
personal style on every commit, and you "can't see the wood
for the trees".<br>
<br>
So if you edit a file and it uses tabs then use tabs too, or
if it "cuddles elses" then do that too. But adapt on a
per-file basis. That way the other team members will better
see (and hopefully appreciate more) your contributions.<br>
<br>
I hope some of this makes sense (maybe I should cut down on
the painkillers...) <br>
<br>
Cheers<br>
------------------------------------------------------------------------------<br>
Benefiting from Server Virtualization: Beyond Initial Workload<br>
Consolidation -- Increasing the use of server virtualization
is a top<br>
priority.Virtualization can reduce costs, simplify management,
and improve<br>
application availability and disaster protection. Learn more
about boosting<br>
the value of server virtualization. <a moz-do-not-send="true"
href="http://p.sf.net/sfu/vmware-sfdev2dev" target="_blank">http://p.sf.net/sfu/vmware-sfdev2dev</a><br>
_______________________________________________<br>
Tsheetx-developers mailing list<br>
<a moz-do-not-send="true"
href="mailto:Tsh...@li...">Tsh...@li...</a><br>
<a moz-do-not-send="true"
href="https://lists.sourceforge.net/lists/listinfo/tsheetx-developers"
target="_blank">https://lists.sourceforge.net/lists/listinfo/tsheetx-developers</a><br>
<br>
</blockquote>
</div>
<br>
<pre wrap=""><fieldset class="mimeAttachmentHeader"></fieldset>
------------------------------------------------------------------------------
Benefiting from Server Virtualization: Beyond Initial Workload
Consolidation -- Increasing the use of server virtualization is a top
priority.Virtualization can reduce costs, simplify management, and improve
application availability and disaster protection. Learn more about boosting
the value of server virtualization. <a class="moz-txt-link-freetext" href="http://p.sf.net/sfu/vmware-sfdev2dev">http://p.sf.net/sfu/vmware-sfdev2dev</a></pre>
<pre wrap=""><fieldset class="mimeAttachmentHeader"></fieldset>
_______________________________________________
Tsheetx-developers mailing list
<a class="moz-txt-link-abbreviated" href="mailto:Tsh...@li...">Tsh...@li...</a>
<a class="moz-txt-link-freetext" href="https://lists.sourceforge.net/lists/listinfo/tsheetx-developers">https://lists.sourceforge.net/lists/listinfo/tsheetx-developers</a>
</pre>
</blockquote>
</body>
</html>
|
