views:

321

answers:

4

Hi I could really use some help with the best way to accomplish the following: I have the below in my controller (and I know this should not be here and needs to move to the model)

This is an email messaging system so according to what position you hold you are able to email out to set groups of people. So if you are Battalion Commander etc you can choose to message out to one of the 5 groups defined below. If you are a Company Commander your groups change. In the view there is a drop down menu and you choose the group your message goes out to. The select menu is populated depending on the position of the signed in user.

The problem seems that the "elsif" portion does not populate the message correctly. It shows the right drop down list and acts as if the email is sent but the emails are not being populated. However the first value (Battalion Commander) works fine.

Do I have something written incorrectly in the if else statement? It seems like it should be pretty simple. The user position always reflects correctly so that is not it.

if (@position == "Battalion Commander" or "Command Sergeant Major" or "FRSA" or "Battalion FRG Leader")
  @bnok = (@user.battalion.primaries).collect(&:email).select{|s| !s.blank?}.join(", ")
  @bspouses = (@user.battalion.primaries(:conditions => ["relationship = 'spouse'"])).collect(&:email).select{|s| !s.blank?}.join(", ")
  @bsoldiers= (@user.battalion.soldiers).collect(&:email).select{|s| !s.blank?}.join(", ")
  @bsoldierspouse=((@user.battalion.soldiers)+(@user.battalion.primaries(:conditions => ["relationship = 'spouse'"]))).collect(&:email).select{|s| !s.blank?}.join(", ")
  @ballcontacts=((@user.battalion.soldiers)+(@user.battalion.primaries)+(@user.battalion.additionals)).collect(&:email).select{|s| !s.blank?}.join(", ")
elsif
  @position == ("Company Commander" or "First Sergeant" or "FRG Leader")
  @nok = (@user.company.primaries).collect(&:email).select{|s| !s.blank?}.join(", ")
  @spouses = (@user.company.primaries(:conditions => ["relationship = 'spouse'"])).collect(&:email).select{|s| !s.blank?}.join(", ")
  @soldiers= (@user.company.soldiers).collect(&:email).select{|s| !s.blank?}.join(", ")
  @soldierspouse=((@user.company.soldiers)+(@user.company.primaries(:conditions => ["relationship = 'spouse'"]))).collect(&:email).select{|s| !s.blank?}.join(", ")
  @allcontacts=((@user.company.soldiers)+(@user.company.primaries)+(@user.company.additionals)).collect(&:email).select{|s| !s.blank?}.join(", ")
end

So this does not work, it work for either one set of positions or the other but not both. This correlates to a select menu in the view and depending on what position you hold the query on certain groups of people change.

So in the view I have this:

<% if @position == "Battalion Commander" %>
<%= f.select(:bcc_email, [["Please Select", ""], ["Battalion NOK", @bnok], ["Battalion Spouses", @bspouses], ["Battalion Soldiers Only", @bsoldiers], ["Battalion Soldiers & Spouses", @bsoldierspouse], ["All Battalion Contacts", @ballcontacts]]) %></h1><br />

I am still learning rails and I am not sure if a case statement would be better but then I am confused on where that goes and how that case statement fits into the view.

Any guidance would be great, I am trying to chip away at this and figure it out, but I would really appreciate some help.

A: 

Firstly it may help if you can format that a bit clearer - for us and yourself, often simple formatting will help identify issues.

Secondly, what is your goal here? I gather it's some sort of war simulator or something? And assume you realise that in an if statement, only one of them will be executed:

if (xxx)
    aaaaaaaaaaaaaaaaaa
elseif (yyyy)
    bbbbbbbbbbbbbb
end

in the case that xxx is true, the aaaaaaaa will be executed, and then it'll jump to end. The bbbbbbbbb part will not be executed - even if true, because the first statement was. If however xxx is not true, yyyy will be evaluated, and if true, bbbbbbbbbbbbb will happen.

I hope that helps a bit?

Mark Mayo
Thanks for your help the issue (I think) is that when (yyy) is the value bbbbbbb is not happening. I just don't know why.
Well can you debug it and print our the value of position at that point and make sure it's one of those three? Even try setting it to one of them directly before the if statement to make sure it has (in theory) no choice but to go into the elseif part.Also - will xxxx or yyyy ALWAYS include the option? I mean - is there any case which will go into neither option? If not, howabout having:if (xxxxx)aaaaaaaaaaaaelsebbbbbbbbbbendifBecause it has to fall into one of them that way regardless.
Mark Mayo
A: 

disclaimer: I don't know ruby. I have been told it is similar to python and LISP in some regards, so this answer makes that assumption.

What I would do is maintain these condition variables in a dictionary (or map, or hash table, whatever your language calls it). The "rank" of the person would be a key, and the value would correspond to the function you want to execute for that rank. for instance,

#the following example is python-esque. you'll have to port it to ruby.
def some_funct1(): #pass parameters if you need to. i don't, here.
    sql_stuff_here

def some_funct2():
    sql_stuff_here

map['important dude'] = some_funct1;
map['another important dude'] = some_funct1;
map['unimportant dude'] = some_funct2;
map['another_unimportant dude'] = some_funct2;

#after some time, you have a person whose rank is rank.

map[rank]() #this will execute the function
San Jacinto
A: 

Sorry, but I don't understand what your code "does".

Could you please say it "in words" instead of in ruby?

In any case, that amount of code certainly looks like belonging to a model.

egarcia
This is not an answer; it is a comment.
San Jacinto
I made some edits.. perhaps that will help.
This is still not an answer. An acceptable attempt at an answer is: "try this to fix it" or "perhaps this solution will help" or "in a similar situation, i did this.."... not "i don't understand your example."
San Jacinto
Well, it answers one part of the question - the "does it belong to the controller?" part. Usually this amount of lines should be on a function on the model.
egarcia
A: 

You wrote

if (... a big test ...)
  ..do some work..
elsif
  ..do something else..
end

but that is wrong! You should just write else instead of elsif unless you need to check another condition.

[I assume elsif followed by nothing=nil is always false]

nathanvda