On Wed, Dec 31, 2008 at 12:02 AM, Jesse Crockett <li...@ruby-forum.com>wrote:
> I guess it speaks for itself. > Yes. It says "I'm too hard to test". More specifically, it has a http://en.wikipedia.org/wiki/Cyclomatic_complexity of ca 14, which I consider very high. I usually try to strive for max 5. Starting to write RSpec for this is starting in the wrong end. This code is so complex (both in terms of readability and number of branches and paths) that trying to wrap a unit test (RSpec spec) around it is going to be a huge headache. I recommend you do some serious extract method refactorings. ( http://www.refactoring.com/catalog/extractMethod.html). While refactoring you should try to push the logic from the controller into the model: http://weblog.jamisbuck.org/2006/10/18/skinny-controller-fat-model This can seem like a chicken and egg problem. How do you refactor without the fear that you have broken something? For this I recommend you slap some Cucumber around this piece of code. It allows you to write tests at a much higher level. When you have 3-10 green Cucumber scenarios for this you can start refactoring without too much fear. Then you can start using RSpec. Teams that get into the habit of writing specs before the code end up with much more readable and maintainable code. :-) Aslak > def bid > auction = Auction.new > p_value = params[:bid][:point] > @auction = Auction.find_by_auction_id(params[:bid][:auction_id]) > > if @completed = (@auction.bid_count == @auction.bid_limit) > flash[:warning] = "Other bidding has completed the auction. Your > bid has been returned." > elsif current_user.credits > 0 > if params[:bid][:point].blank? > flash[:error] = "Sorry, you must enter a valid bid." > elsif p_value.to_i < 1 or p_value.to_i > > auction.point_range(params[:bid][:auction_id]) > flash[:error] = "Sorry, you attempted to place a bid out of > range." > elsif not Bid.find_by_user_id_and_point(params[:bid][:user_id], > params[:bid][:point]).nil? > flash[:error] = "You have already placed this point > (#{p_value.to_i})." > else > #place bid > @bid = Bid.new(params[:bid]) > point = @bid.point > if @bid.save > # update winning charities and pref charity places > > # TODO make sure that the top bids are used for place (double > check) > > update_charities_on_bid(current_user, @auction) > > # set auction winners > if @completed > winners = Bid.new.winners(params[:bid][:auction_id]) > a.winner_first_id = winners.first[:user_id] > a.winner_second_id = winners.second[:user_id] > a.winner_third_id = winners.third[:user_id] > a.duration_hours = ((Time.now - a.scheduled_start) / 60.0 / > 60.0).to_i > end > > current_user.credits -= 1 > current_user.save > > user_bids = > Bid.find_all_by_user_id_and_auction_id(current_user.id, > @auction.auction_id).count > if user_bids == 1 > @auction = > Auction.find_by_auction_id(params[:bid][:auction_id]) > @auction.total_bidders += 1 > end > @auction.bid_count += 1 > @auction.save > > if Bid.find_all_by_point(point).count > 1 > flash[:notice] = "Auction Completed. We have winners!!" if > @completed > flash[:warning] = "Oh! Someone else has bid the same point. > Please try again." if not @completed > flash[:warning] = "Bravery! However, your bid was not > unique. Please view the auction results." if @completed > else > flash[:notice] = "Success! You bid for point #{point}. It > is unique!" > flash[:notice] << " Auction Completed. We have winners!!" > if @completed > end > else > flash[:error] = "Bleep, bloop. System error, please try > again." > end > end > else > flash[:error] = "Whoops, you have zero credits available to bid." > end > redirect_back_or_default :show > end > > def update_charities_on_bid(user, auction) > # best_bids: top 3 winning bids > best_bids = Bid.all.uniq > best_bids.sort! {|a, b| b.point <=> a.point}[0..2] > > best_bids[1] = 0 if best_bids.second.nil? > best_bids[2] = 0 if best_bids.third.nil? > > # TODO check timestamp for place ?? > > if best_bids.first.user_id == user.id > auction.charity_first_id = user.pref_charity_one_id > elsif best_bids.second != 0 && best_bids.second.user_id == user.id > auction.charity_second_id = user.pref_charity_two_id > elsif best_bids.third != 0 && best_bids.third.user_id == user.id > auction.charity_third_id = user.pref_charity_three_id > end > > auction.save > end > > > -- > Posted via http://www.ruby-forum.com/. > _______________________________________________ > rspec-users mailing list > rspec-users@rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
_______________________________________________ rspec-users mailing list rspec-users@rubyforge.org http://rubyforge.org/mailman/listinfo/rspec-users
