current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 2 down vote favorite

I have this for loop for creating a list of variables:

vars <- c( 'beta.o', 'sd.y') 
for (x in c('ghs', 'site', 'trt')) {
  if(model.parms[[x]] == 1) {
    data <- data[, which(names(data) != x)]
  } else {
    data <- data
    if(x!='ghs') {
      vars <- c(vars, paste('sd.', x, sep = ''))
    }
    m <- min(model.parms[[x]], 5)
    for (i in 1:m) {
      if(i == 1 && x == 'site') {
        vars <- c(vars, 'beta.site[1]')
      }
      if (i > 1) {
        vars <- c(vars, paste('beta.', x, '[', i, ']', sep=''))
      }
    }
  }
}

It has been bothering me terribly, and I have failed the last two times I have tried to replace it, although conceptually it should be able to be written in a few lines. Any advice?

share|improve this question
up vote 3 down vote

This bit:

for (i in 1:m) {
  if(i == 1 && x == 'site') {
    vars <- c(vars, 'beta.site[1]')
  }
  if (i > 1) {
    vars <- c(vars, paste('beta.', x, '[', i, ']', sep=''))
  }
}

Says handle the first pass differently from all the others. So I would replace it with this:

if (x == 'site') {
  vars <- c(vars, 'beta.site[1]')
}
for (i in 2:m) {
  vars <- c(vars, paste('beta.', x, '[', i, ']', sep=''))
}
share|improve this answer
up vote 1 down vote

The big mistake in your code is you are dynamically growing your vectors. For example, compare

x = NULL 
for(i in 1:100000)
   x = c(x, i)

with 

x = numeric(100000)
for(i in 1:100000) 
   x[i] = i

The other key point is that paste function can be vectorised. So, 

for (i in 1:m) {
  if(i == 1 && x == 'site') {
    vars <- c(vars, 'beta.site[1]')
   }
   if (i > 1) {
     vars <- c(vars, paste('beta.', x, '[', i, ']', sep=''))
   }
 }

can be replaced with

if(x == 'site') {
    vars <- c(vars, 'beta.site[1]')
   }
vars = c(vars, paste('beta.', x, '[', i, ']', sep=''))
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.