[rspec-users] My first tests, backwards. Please help

aslak hellesoy aslak.hellesoy at gmail.com
Tue Dec 30 18:29:06 EST 2008


On Wed, Dec 31, 2008 at 12:02 AM, Jesse Crockett <lists at 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 at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://rubyforge.org/pipermail/rspec-users/attachments/20081231/8aa2817c/attachment.html>


More information about the rspec-users mailing list