On Apr 14, 7:55 am, Nilesh K. [email protected]
wrote:
error, unexpected tCONSTANT
[…]
OK, after some investigation, it looks like you have an unmatched
brace on line 32, which is exactly the sort of thing I suspected.
By the way, your view file contains some of the worst view code I’ve
ever seen – it contains way too much logic, and much of that logic
is poorly written. Some suggestions for fixing it up:
You probably don’t need this. If I understand correctly what you’re
doing, you should probably use respond_to in the controller to set up
different views.
%script
Don’t do that. Learn about Haml filters and use :javascript instead.
Better yet, get rid of the inline JavaScript – put it in a separate
file.
-
@ShareContentTitle = “#{@track.title} from album #{@album.title} at aflatune!”
This should be a method on the Track model.
- trackIndex = 0;
- for track in @album.tracks
- suffix = “track_#{@track.to_param}album#{@album.id}track#{trackIndex}”;
%li.SongListItem
= track_link_html(suffix ,track.to_param, track.title)
- trackIndex = trackIndex + 1
This is not properly indented, so it will not work (the entire body of
the for loop should be indented). But even if it were properly
indented, it would be a bad way of doing things. Ruby is not PHP –
don’t write it as if it were. In particular, in Ruby, it should
never be necessary to manually increment a loop index as you’re
doing here trackIndex. What you want is something like this:
-
@album.tracks.each_with_index do |track, i|
%li.SongListItem= track_link_html(“track_#{@track.to_param}album#
{@album.id}track#{i}” ,track.to_param, track.title)
(yes, that’s two lines instead of six!)
Also note that in Ruby, it is considered poor style to use CamelCase
variable names such as trackIndex. The usual way of writing this
would be track_index. You also don’t need the semicolons at the end
of a line, and most people don’t use them. Please take the time to
study some good Ruby style.
-
@phrase = nil;
-
@video_id = @track[:bare_id]
-
@videoSearchHeadingTag = ‘h3’;
-
@videoSearchHeading = ‘Related videos’;
-
@showLongVideoResults = true
= render :partial => ‘home/video_search’
Setting this many variables in the view is a sure sign that something
is wrong. These should either be model methods or set in the
controller.
Alternatively, if you’re passing these values into the partial, please
read up on proper render :partial syntax, in particular the use
of :locals. You probably want something like
render :partial => ‘partial’, :locals => {:video_id =>
@track.bare_id, :another_variable => ‘another value’, :next_variable
=> ‘and so on’…}
- elsif
%h4
Sorry, the track could not be located in our system.
You have an elsif without a condition. Did you mean else? And why an
h4 when there’s no h3 to nest it in?
Let me know if you have any further questions. But if you’re going to
work with Rails, please take the time to learn the Rails way of
doing things – you will be much happier.
Best,
Marnen Laibow-Koser
[email protected]
http://www.marnen.org