# Would someone like to tell me why this code will not solve m

**URL:** <https://www.ruby-forum.com/t/would-someone-like-to-tell-me-why-this-code-will-not-solve-m/88292>\
**Category:** Rails\
**Created:** [January 3, 2006, 5:12am UTC](https://www.ruby-forum.com/t/would-someone-like-to-tell-me-why-this-code-will-not-solve-m/88292 "2006-01-03T05:12:33Z")\
**Posts on this page:** 5\
**Page:** 1

<div class="post-metadata">

**Author:** ![brucebalmer](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/3ec8ea/32.png) [@brucebalmer](https://www.ruby-forum.com/u/brucebalmer)\
**Post date:** [January 3, 2006, 5:12am UTC](https://www.ruby-forum.com/t/would-someone-like-to-tell-me-why-this-code-will-not-solve-m/88292/1 "2006-01-03T05:12:33Z")

</div>

I am building an invoicing system but cannot use the auto\_increment  
field to determine the invoice number (because they are running 3  
different companies off the one system. I need to find the last  
invoice number from any given company and then add 1 to it to get the  
next invoice number. BUT, there is a unique case on the very first  
invoice produced because there is no earlier invoice number. My  
solution is as follows (and does not work)

This is in the ‘create’ controller

```
 if Invoice.find(:all)
   @temp = Invoice.find_by_sql("select max(inv_number) as inv_number
                           from invoices where company_id = params

```

[:company\_id")  
@invoice.inv\_number = @temp[0].inv\_number +1  
else  
@invoice.inv\_number=1  
end

The error I am getting is

NoMethodError in Invoices#create

You have a nil object when you didn’t expect it!  
You might have expected an instance of Array.  
The error occured while evaluating nil.+

---

<div class="post-metadata">

**Author:** ![brucebalmer](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/3ec8ea/32.png) [@brucebalmer](https://www.ruby-forum.com/u/brucebalmer)\
**Post date:** [January 3, 2006, 5:25am UTC](https://www.ruby-forum.com/t/would-someone-like-to-tell-me-why-this-code-will-not-solve-m/88292/2 "2006-01-03T05:25:57Z")

</div>

bruce balmer wrote:

> if Invoice.find(:all)  
> @temp = Invoice.find\_by\_sql(“select max(inv\_number) as inv\_number  
> from invoices where company\_id  
> = params[:company\_id”)

This line might be part of the problem. the “params[:company\_id]” is  
being inserted literally into your sql string. You need to do something  
like

… = #{params[:company\_id]}" to insert it.

WARNING… this is highly unsafe. A user could manually pass all sorts  
of badness through the company\_id param and totally hose you. Please  
find some other (secure) way to do this. The [‘company\_id = ?’,  
params[:company\_id]] approach is better, but I’m not sure how to do this  
with the find\_by\_sql function.

By the way, you could do something like…

Invoice.find(:first, :condition=\>[“company\_id = ?”,  
params[:company\_id]], :order=\>“inv\_number DESC”)

This should give you the record with the highest invoice number.

\_Kevin

---

<div class="post-metadata">

**Author:** ![brucebalmer](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/3ec8ea/32.png) [@brucebalmer](https://www.ruby-forum.com/u/brucebalmer)\
**Post date:** [January 3, 2006, 5:48am UTC](https://www.ruby-forum.com/t/would-someone-like-to-tell-me-why-this-code-will-not-solve-m/88292/3 "2006-01-03T05:48:50Z")

</div>

On Mon, Jan 02, 2006 at 09:10:47PM -0700, Bruce B. wrote:

> This is in the ‘create’ controller

Invoice.find(:all) will always be boolean true. If there are no  
Invoices,  
then find(:all) will return an empty array ([]), which is always boolean  
true.

A way of expressing your intentions is:

unless Invoice.count.zero?

> ```
> if Invoice.find(:all) 
> 
> ```

params[:company\_id] is not being interpolated. It’s being passed along  
as a  
literal value. String interpolation in Ruby is done with #{}. For  
example:

foo = ‘Marcel’  
“My name is #{foo}”  
=\> “My name is Marcel”

The following find\_by\_sql, makes you vulnerable to SQL injection. Rather  
than  
using find\_by\_sql, you could use find, passing the :select and the  
:conditions options. Something like:

Invoice.find(:first, :select =\> ‘MAX(inv\_number) as inv\_number’,  
:conditions =\> [‘company\_id = ?’, params[:company\_id]).inv\_number.to\_i

But there is no need to do this MAX business. You can instead use  
ActiveRecord’s increment or increment! or increment\_counter method:

[http://api.rubyonrails.com/classes/ActiveRecord/Base.html#M000768](http://api.rubyonrails.com/classes/ActiveRecord/Base.html#M000768)  
[http://api.rubyonrails.com/classes/ActiveRecord/Base.html#M000727](http://api.rubyonrails.com/classes/ActiveRecord/Base.html#M000727)

Increment allows you to avoid the entire conditional.

> ```
> @temp = Invoice.find_by_sql("select max(inv_number) as inv_number
> from invoices where company_id =
> 
> ```
> 
> params[:company\_id")  
> @invoice.inv\_number = @temp[0].inv\_number +1  
> else  
> @invoice.inv\_number=1  
> end

marcel

---

<div class="post-metadata">

**Author:** ![brucebalmer](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/3ec8ea/32.png) [@brucebalmer](https://www.ruby-forum.com/u/brucebalmer)\
**Post date:** [January 4, 2006, 4:16am UTC](https://www.ruby-forum.com/t/would-someone-like-to-tell-me-why-this-code-will-not-solve-m/88292/4 "2006-01-04T04:16:16Z")

</div>

Marcel:

Thanks very much. That was awesome!

bruce

---

<div class="post-metadata">

**Author:** ![brucebalmer](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/3ec8ea/32.png) [@brucebalmer](https://www.ruby-forum.com/u/brucebalmer)\
**Post date:** [January 3, 2006, 10:27am UTC](https://www.ruby-forum.com/t/would-someone-like-to-tell-me-why-this-code-will-not-solve-m/88292/5 "2006-01-03T10:27:34Z")

</div>

On 3.1.2006, at 6.55, Marcel Molina Jr. wrote:

> But there is no need to do this MAX business. You can instead use  
> ActiveRecord’s increment or increment! or increment\_counter method:
> 
> [Peak Obsession](http://api.rubyonrails.com/classes/ActiveRecord/Base.html#M000768)  
> [Peak Obsession](http://api.rubyonrails.com/classes/ActiveRecord/Base.html#M000727)
> 
> Increment allows you to avoid the entire conditional.

Also keep in mind that selecting max() from a table is NOT a  
transaction safe method. If you have two users inserting an invoice  
concurrently, they might end up with the same numbe. That’s why many  
database management systems have sequences [1].

//jarkko

[1] [http://www.postgresql.org/docs/8.1/interactive/sql-](http://www.postgresql.org/docs/8.1/interactive/sql-)  
createsequence.html