I'll provide feedback from top to bottom:
1. elasticity<-function(y.p,x.p,band){
It's recommended to use spaces to make your code easier on the eyes. Spaces also avoid ambiguous situations for people who are not experts in R's syntax rules; for example, when someone sees a<-b
, is it assigning b
to a
, or is it asking if a
is less than -b
? Maybe another way to convince you is to make you notice that R's compiled code does contain spaces (run print(lapply)
for example); it shows that R's authors agreed that code readability was more important than compactness.
Now about the choice of arguments. Why not use the same argument names (and order) as locpoly
since it is the main function being called here? So I'd recommend x, y, bandwidth
. Or maybe x.price, y.price, bandwidth
if it really helps knowing these are prices.
2. require(KernSmooth)
To convince you to use library
instead of require
, check http://www.r-bloggers.com/library-vs-require-in-r/.
3. as.vector(y.p);as.vector(x.p)
These two calls to as.vector
do nothing if you do not assign the output. You probably meant to do:
y.p <- as.vector(y.p)
x.p <- as.vector(x.p)
11. y1<-as.vector(loess.der$y)
If you check using is.vector()
, you will see that loess.der$y
is already a vector, so as.vector
is unnecessary.
12. eps<-matrix(NA,length(y.p),2)
Would a data.frame make more sense? Or if you really want a matrix, should use be using cbind
?
14. for(i in 1:length(y.p)){
15. eps[i,2]<-x.p[i]/y.p[i]*y1[i]
16. }
Here, you are really missing up on vectorization. This should be:
eps[, 2] <- x.p / y.p * y1
Overall, if defining a data.frame, appreciate the simplicity and readability of:
eps <- data.frame(Price = x.p,
Elasticity = x.p / y.p * loess.der$y)
Or if you want a matrix, just replace data.frame()
with cbind()
.
Taking all of my comments into account, your code could look like this:
elasticity <- function(x, y, bandwidth) {
library(KernSmooth)
x <- as.vector(x)
y <- as.vector(y)
loess.fun <- locpoly(x = x, y = y, bandwidth = bandwidth)
plot(loess.fun, type = 'l', lwd = 2, lty = 4,
ylim = c(0, 100), xlab = "Price", ylab = "Share", col = 2,
main = "Local Polynomials Estimation")
loess.der <- locpoly(x = x, y = y, drv = 1, bandwidth = bandwidth,
gridsize = length(y))
eps <- data.frame(Price = x,
Elasticity = x / y * loess.der$y)
return(eps[order(eps$Price), ])
}