Implementation of thread-safe vector I have implemented a thread safe vector, based on std::thread and std::vector. I am new to many of the patterns used in threading and was wondering if there's a more efficient way the locking can be implemented? Also I'm not sure what the best name is to give variables that just get stored right away (i.e. in).

Here is SafeVector.hpp

#ifndef SAFEVECTOR_HPP
#define SAFEVECTOR_HPP

#include <vector>
#include <mutex>
#include <condition_variable>
#include <string>

class SafeVector {
public:
    SafeVector() : vec(), mut(), cond() {}
    SafeVector(const SafeVector& orig) : vec(orig.vec), mut(), cond() {}
    ~SafeVector() {}

    void insert(std::string in, const int index);
    void push_back(std::string in);
    std::string& operator[](const int index);
    std::vector<std::string>::iterator begin();
    std::vector<std::string>::iterator end();
    std::vector<std::string> toVector();

private:
    std::vector<std::string> vec;
    std::mutex mut;
    std::condition_variable cond;
};

#endif /* SAFEVECTOR_HPP */

Here is SafeVector.cpp

#include <string>
#include <utility>
#include "SafeVector.hpp"

void SafeVector::insert(std::string in, const int index)
{
    std::lock_guard<std::mutex> lock(mut);
    vec[index] = std::move(in);
    cond.notify_one();
}

void SafeVector::push_back(std::string in)
{
    std::lock_guard<std::mutex> lock(mut);
    vec.push_back(std::move(in));
    cond.notify_one();
}

std::string& SafeVector::operator[](const int index)
{
    return vec[index];
}

std::vector<std::string>::iterator SafeVector::begin()
{
    return vec.begin();
}

std::vector<std::string>::iterator SafeVector::end()
{
    return vec.end();
}

std::vector<std::string> SafeVector::toVector()
{
    return vec;
}

Currently, these are the only functions I need to use on vectors but if I decide to implement more, I'm assuming the rest would be done in the same way.

We probably want to be able to move-construct, so as well as this constructor:

SafeVector(const SafeVector& orig) : vec(orig.vec), mut(), cond() {}

We probably want also:

SafeVector(SafeVector&& orig)
  : vec{std::move(orig.vec)},
    mut{},
    cond{}
{}

I'd recommend being able to construct from a standard vector:

SafeVector(std::vector vec)
  : vec{std::move(vec)},
    mut{},
    cond{}
{}

Since we always default-initialize mut and cond, we could declare their initializers in the class definition, removing the need to have them in the constructors' initializer lists.

There's no need to specify an empty destructor - just let the compiler generate it.

I think we probably want one or more assignment operators.

Big question: how can we safely use iterators we get from begin() and end()?  There's no way for client code to lock the mutex until it's finished using the iterators.


Nginx cache path function I have an MD5 hash string:

_hash = '743a0e0c5c657d3295d347a1f0a66109'

I want to write function that splits passed string to path in nginx cache format:

print hash2path(_hash, [1, 2, 3])
# outputs '7/43/a0e/743a0e0c5c657d3295d347a1f0a66109'

This is my variant:

def hash2path(_hash, levels):
    hash_copy = _hash
    parts = []
    for level in levels:
        parts.append(_hash[:level])
        _hash = _hash[level:]
    parts.append(hash_copy)
    return '/'.join(parts)

How could this be improved?

Why you can't just hardcode cache path calculation?

>>> _hash = '743a0e0c5c657d3295d347a1f0a66109'
>>> def hash2path(_hash):
...     return "%s/%s/%s/%s" % (_hash[0], _hash[1:3], _hash[3:6], _hash)
...
>>> hash2path(_hash)
'7/43/a0e/743a0e0c5c657d3295d347a1f0a66109'

UPDATE: If you really need to pass level array there is a little bit faster version:

def hash2path(_hash, levels):
    prev = 0
    parts = []
    for level in levels:
        parts.append(_hash[prev:prev + level])
        prev += level
    parts.append(_hash)
    return "/".join(parts)


Optimize LINQ search with custom fixed ranking I have to perform on some collection and return ranking based on some logic
I wish to optimize this working code since I think it can be bettered (maybe using tasks?).

I need to search on Counterparts item. I load this data from DB and I've got more aliases per counterpart.

I need to return the result based on those criteria:

Counterpart Code is equal to search string
Counterpart Code starts with search string
Counterpart Code contains search string
Counterpart Description contains search string
Counterpart Alias is equal to search string
Counterpart Alias starts with search string
Counterpart Alias contains search string

Each of those rules starts from Ranking 1 to 7 and I have to sort ok that ranking ascending.

public class Counterpart
{
    public int Id { get; set; }

    public string Code { get; set; }

    public string Description { get; set; }

    public IEnumerable<Alias> Aliases { get; set; }

    public override bool Equals(object obj)
    {
        Counterpart obj2 = obj as Counterpart;
        if (obj2 == null) return false;
        return Id == obj2.Id;
    }
}

public class Alias
{
    public int? Type { get; set; }

    public string Description { get; set; }
}

internal class CounterPartRanking
{
    public int Rank { get; set; }

    public Counterpart CounterPart { get; set; }
}

public static class CounterpartExtensions
{
    public static IEnumerable<Counterpart> SearchWithRank(this IEnumerable<Counterpart> source, string pattern)
    {
        var items1 = source.Where(x => x.Code == pattern);
        var items2 = source.Where(x => x.Code.StartsWith(pattern));
        var items3 = source.Where(x => x.Code.Contains(pattern));
        var items4 = source.Where(x => x.Description.Contains(pattern));
        var items5 = source.Where(x => x.Aliases != null && x.Aliases.Any(y => y.Description == pattern));
        var items6 = source.Where(x => x.Aliases != null && x.Aliases.Any(y => y.Description.StartsWith(pattern)));
        var items7 = source.Where(x => x.Aliases != null && x.Aliases.Any(y => y.Description.Contains(pattern)));

        Stopwatch sw = Stopwatch.StartNew();
        var rankedItems = new List<CounterPartRanking>();

        if (items1.Any())
            rankedItems.AddRange(items1.Select(x => new CounterPartRanking { Rank = 1, CounterPart = x }));
        if (items2.Any())
            rankedItems.AddRange(items2.Select(x => new CounterPartRanking { Rank = 2, CounterPart = x }));
        if (items3.Any())
            rankedItems.AddRange(items3.Select(x => new CounterPartRanking { Rank = 3, CounterPart = x }));
        if (items4.Any())
            rankedItems.AddRange(items4.Select(x => new CounterPartRanking { Rank = 4, CounterPart = x }));
        if (items5.Any())
            rankedItems.AddRange(items5.Select(x => new CounterPartRanking { Rank = 5, CounterPart = x }));
        if (items6.Any())
            rankedItems.AddRange(items6.Select(x => new CounterPartRanking { Rank = 6, CounterPart = x }));
        if (items7.Any())
            rankedItems.AddRange(items7.Select(x => new CounterPartRanking { Rank = 7, CounterPart = x }));

        sw.Stop();

        Debug.WriteLine("Time elapsed {0} for {1}", sw.Elapsed, pattern);
        var items = rankedItems.OrderBy(x => x.Rank).Select(x => x.CounterPart);
        var distinct = items.Distinct();

        return distinct;
    }
}

Distinct

Take a look at the documentation for Distinct.

A couple things from the remarks should stand out for you:

  The Distinct(IEnumerable) method returns an unordered sequence that contains no duplicate values.

I know that the order of the sequence is preserved when using Distinct but that's an implementation detail - there's no gaurantee.

  The default equality comparer, Default, is used to compare values of the types that implement the IEquatable generic interface. To compare a custom data type, you must implement this interface and provide your own GetHashCode and Equals methods for the type.

Your custom type Counterpart does not do this. It should implement IEquatable<Counterpart> and additionally override both GetHashCode and Equals.

public class CounterPart : IEquatable<Counterpart>
{
    // ...
    public bool Equals(Counterpart other)
    {
        // left for you.
    }

    public override bool Equals(object other)
    {
        return Equals(other as Counterpart);
    }

    public override bool GetHashCode()
    {
         // http://stackoverflow.com/a/263416/1402923
    }
}

Now you have a type that is safe for use with Distinct!

AddRange

As far as I know, AddRange only throws when the collection is null. You don't need to check for items before you call it. That elminates a whole heap of code:

rankedItems.AddRange(items1.Select(x => new CounterPartRanking { Rank = 1, CounterPart = x }));
rankedItems.AddRange(items2.Select(x => new CounterPartRanking { Rank = 2, CounterPart = x }));
rankedItems.AddRange(items3.Select(x => new CounterPartRanking { Rank = 3, CounterPart = x }));
rankedItems.AddRange(items4.Select(x => new CounterPartRanking { Rank = 4, CounterPart = x }));
rankedItems.AddRange(items5.Select(x => new CounterPartRanking { Rank = 5, CounterPart = x }));
rankedItems.AddRange(items6.Select(x => new CounterPartRanking { Rank = 6, CounterPart = x }));
rankedItems.AddRange(items7.Select(x => new CounterPartRanking { Rank = 7, CounterPart = x }));

Other comments

As I mentioned previously, this isn't gauranteed to be correct (but is in every instance I know of):

var items = rankedItems.OrderBy(x => x.Rank).Select(x => x.CounterPart);
var distinct = items.Distinct();


Multiple emails and attachments, reusing memory stream I am generating a pdf form based on user input from an MVC website form. The pdf is genrated using the Foxit .NET SDK, then attached to three separate emails, generated using Postal. It has been requested that the file not be saved to disk at any stage.

I have not worked a lot with files or MemoryStreams before so would just like to know if my code is following the standard practices for managing memory and performance etc.

MergeDocument document = new MergeDocument(); // The PDF File Object
// ... Populate and create the pdf file

// Generate and Send the emails
// To the user
dynamic useremail = new Email("FormSubmitted");
useremail.To = User.EmailAddress;
useremail.Attach(new Attachment(new MemoryStream(document.Draw()), "OrderForm.pdf"));
useremail.Send();

// To the Vendor
dynamic vendoremail = new Email("FormSubmittedVendor");
vendoremail.To = Vendor.EmailAddress;
vendoremail.Attach(new Attachment(new MemoryStream(document.Draw()), "OrderForm.pdf"));
vendoremail.Send();

// To us
dynamic internalemail = new Email("FormSumbittedUs"); // To: is hardcoded in view.
internalemail.Attach(new Attachment(new MemoryStream(document.Draw()), "OrderForm.pdf"));
internalemail.Send();

Is it ok to create new MemoryStream objects for each email?

Basically it is ok, to create a new MemoryStream 3 times, but I have to ask, why you don't create one Attachment adding it to each of the emails like so

var attachment = new Attachment(new MemoryStream(document.Draw()), "OrderForm.pdf");
useremail.Attach(attachment);
vendoremail.Attach(attachment);
internalemail.Attach(attachment);


Drawing a gnome in Logo I recently found my first program I wrote using Logo and was wondering if it could have been improved:

to gnome
    repeat 180 [bk 1 rt 1] repeat 180 [bk 1 rt 1/4]
    rt 97 repeat 180 [bk 1 rt 1/4] pu lt 90 fd 50
    lt 90 fd 25
    rt 90 pd repeat 90 [fd 1/2 rt 1/4]
    repeat 180 [bk 1/2 lt 1/4] pu rt 60 fd 50 pd
    repeat 360 [fd 1/4 rt 1] lt 60 pu fd 25 pd
    repeat 360 [fd 1/4 rt 1] pu rt 90 fd 50 lt 90 fd 12 rt 180
    pd pu bk 20 pd lt 10 repeat 240 [fd 1/2 rt 1/2] pu
    fd 50 rt 90 fd 90 rt 180 pd pu lt 30 fd 110 lt 45 fd 10 setfillcolour [0 204 0]
    fill pu lt 45 fd 40 setfillcolour [243 143 133] fill
end

I think the only thing I do not remember the reason for is the repeats of pu pd but I think it was to ensure that the pen is up.

Readability would be greatly enhanced by splitting the code into stanzas, using more line breaks, and adding comments.

Why do you like drawing backwards?  Going forward is much easier to understand.  The first two lines inside the procedure, for example, could be replaced by this code, which draws a counterclockwise path going forward, rather than a clockwise going backward:

to gnome
    ; Outline of conehead
    rt 7
    repeat 180 [fd 1 lt 1/4]
    lt 97
    repeat 180 [fd 1 lt 1/4]
    repeat 180 [fd 1 lt 1]

    pu lt 83 fd 50
    …

There would then be an extra rt 7 command, and it finishes with lt 83 instead of lt 90, but I think that that's a good thing, because it emphasizes that the shape is skewed, not symmetrical.

Some initialization code would be a good idea, to ensure that the turtle is in an expected state.  For example, ht pd would hide the turtle and put the pen down.  You might also want to set the heading and colour as well.

I don't know what dialect of Logo you are using, but setfillcolour instead of setfillcolor seems unusual.


Loading Configurations from plist into singleton In my iOS application, I've created a singleton class that reads a configuration plist file and provides accessor methods to easily retrieve the values:

class Configuration{

    struct Key{
        static let searchRadius = "searchRadius"
        static let significantUserMovementThreshold = "movementThreshold"
    }

    enum ConfigurationError : ErrorType{
        case FileNotFound
    }

    static let sharedInstance = Configuration()
    var configDictionary : NSDictionary?

    func loadConfigurationFromPropertyList() throws{

        if let filePath = NSBundle.mainBundle().pathForResource("Config", ofType: "plist"){
            self.configDictionary = NSDictionary(contentsOfFile: filePath)
        }else{
            throw ConfigurationError.FileNotFound
        }

    }

    func searchRadius()-> Double?{
        return configDictionary?.objectForKey(Key.searchRadius) as? Double
    }

    func significantUserMovementThreshold() -> Double?{
        return configDictionary?.objectForKey(Key.significantUserMovementThreshold) as? Double
    }

}

In the AppDelegate, I call Configuration.loadConfigurationFromPropertyList to load the configurations into memory.

I have a number of questions regarding this approach:

Is it a good idea to have accessor method for each property in the configuration file? I think it provides a cleaner and more reliable approach than using subscripts.
Is it better for the Configuration class to read the file in its constructor or that AppDelegate invoke loadConfigurationFromPropertyList()?

I don't have a whole lot to add, as this is pretty straightforward and seems like a completely reasonable way to do what you're trying to do.

In regards to your questions:

There's a tradeoff for using accessor methods for properties. Every time you add a new one, you need to add a constant to the Key struct, and add an accessor method to the Configuration class to get its value. If you misspell either of them (in the plist or the string in the Key struct), you won't have the compiler to tell you so, and you can end up with some odd bugs that become hard to track down. However, I personally prefer the way you're doing it because it's easier to read for the future. Your set of configurations is currently fairly small, so it's pretty easy to maintain. I'd go with what you've got unless you think there will be a lot of configuration options that will be controlled by a plist file.
It's better to read the file in the constructor in my opinion. This is known as RAII - Resource Acquisition is Initialization. The idea being that if you do it in the constructor, if it fails, you don't end up with a partially initialized object.


Project Euler #3 with Python I tried to solve some Euler's problems, this is the solution I found for problem #3 with Python. The problem asks:

  The prime factors of 13195 are 5, 7, 13 and 29. What is the largest prime factor of the number 600851475143 ?

How can I improve my code?

def get_largest_prime2(value):
    start = time.clock() #used to measure the execution time
    divisors = []  #this list will be filled with all number's divisors
    i = 1
    while value != 1:
        while value%i == 0: #check if the value can be divided by i and, if so, iterate the process.
            value = (value // i)
            divisors.append(i)
            if i == 1 : break #it's needed to avoid an infinity loop
        if i >= 3: i += 2 #when i is bigger than 3, search divisors among odd numbers( this reduces the search field)
        else: i += 1
    print( time.clock()-start)
    return max(divisors) #returns the max prime divisors

timing

Put the code to time the run time of the function outside of the function

augmented assignment

value = (value // i) can be expressed as value //= i

divisors

Why do you need to keep a list of divisors. They only as for the largest one. The largest one will always be the last one, so you just need to remember this

pep-8

your variable names are clear and in the correct format, apart from i. I would rename that to divisor or prime
The spacing around the operators and ( is inconsistent
the if i >= 3: i += 2 on one line is advised against

increments

    if i >= 3: i += 2 #when i is bigger than 3, search divisors among odd numbers( this reduces the search field)
    else: i += 1

can be simplified to i += (2 if i >= 3 else 1) or even i += 1 + (i >= 3), using the fact that int(True) is 1. But here you don't use the fact that your divisors are all primes. A quicker process would be to get the next possible divisor from a prime generator, instead of incrementing by 2 manually

while loop

You can integrate the if i == 1 : break into the condition for the while loop: while value % prime == 0 and value != 1:

def largest_prime_divisor(value):
    largest_divisor = 1
    for prime in prime_generator():
        while value % prime == 0 and value != 1:
            value //= prime
            largest_divisor = prime
    return largest_divisor

Possible implementations of prime_generator can be found in dozens of SO and Code review posts

shortcut

If the divisor is larger than the square root of the value, you know the next divisor will be the value itself, so you can shortcut when the divisor exceeds this

def largest_prime_divisor(value):
    largest_divisor = 1
    for prime in prime_generator():
        while value % prime == 0:
            value //= prime
            largest_divisor = prime
        if prime ** 2 > value:
            return max(largest_divisor, value)
    return largest_divisor


Mapping JSON object to HTML This is a continuation of previous questions. I just started using react and I've managed to fetch a xml file, convert it to json and then loop through the data. But I think there should be a better way to go through what I've done as I'm using a function I found on another SO answer and then used a map() within it.

Anyways here's the breakdown of my code.

Fetching the xml and converting to json:

componentDidMount() {
    axios.get('https://gist.githubusercontent.com/eMatsiyana/e315b60a2930bb79e869b37a6ddf8ef1/raw/10c057b39a4dccbe39d3151be78c686dcd1101aa/guestlist.xml')
    .then(res => {
      const xml = XMLMapping.load(res.data);
      var guests  = XMLMapping.tojson(xml);
      this.setState({guests: guests});
    });
  }

The results of the json in console:

Object{
  dataset{
   record{
    0{
      company{
        $t: "Skippad"
      }
      first_name{
        $t: "Keith"
      }
      last_name{
        $t: "Cook"
      }
    }
    1{
      company{
        $t: "Skippad"
      }
      first_name{
        $t: "Keith"
      }
      last_name{
        $t: "Cook"
      }
    }
   }
  }
}

I'm using this function to map the object and then using a map() within it:

function mapObject(object, callback) {
  return Object.keys(object).map(function (key) {
    return callback(key, object[key]);
  });
}

This is what the final mapping of the data looks like:

{mapObject(this.state.guests, (key, value) => {
  return

    {value.record
    .filter(
      (item,index) => {
        return (
          item.first_name.$t.toLowerCase().includes(this.state.search.toLowerCase())
          //item.last_name.$t.toLowerCase().includes(this.state.search.toLowerCase())
          //item.company.$t.toLowerCase().includes(this.state.search.toLowerCase())
        )
      }
    )
    .map((item,index) => {
      return

{item.first_name.$t} {item.last_name.$t}
{item.company.$t}

{item.company.$t}

          <EmailFormdisplay guestid={index} />

          <PhoneFormdisplay guestid={index} />

          <Dropdown>
            <DropdownTrigger></DropdownTrigger>
              <DropdownContent>

                    <EmailFormdisplay guestid={index} />

                    <PhoneFormdisplay guestid={index} />

              </DropdownContent>
          </Dropdown>

;
    })}

  })}

Is there a better way of doing this without using mapObject() and a map() within it?

Any kind of feedback or advice would be greatly appreciated!

A few things:

I'm not sure why you need to map over the top level object? Are there top-level keys other than dataset that
you're interested in? From your example, it looks like you only need the array of records (guests), in which case you don't need mapObject.
I usually try to keep JSX expressions as slim as possible. Your JSX is quite long, and I can see 3 opportunities to shorten/simplify it:

Push more of the transformation into componentDidMount, making the data easier to work with during render
If some transformation has to be done during render (e.g. the search filtering), then consider assigning it to an intermediate local variable rather than doing it inline in JSX
Extract smaller components for logical sections

I'm guessing that those commented out lines were meant to be || (or) clauses?
Try naming variables according to the domain model. I.e., guest rather than item.
There's a bit of duplication that can be reduced, like lower-casing the search string.

Here's a complete example (haven't tested it!):

class Guests extends React.Component {
  constructor(props) {
    super(props);
    this.state = { guests: [] };
  }

  componentDidMount() {
    axios.get('https://gist.githubusercontent.com/eMatsiyana/e315b60a2930bb79e869b37a6ddf8ef1/raw/10c057b39a4dccbe39d3151be78c686dcd1101aa/guestlist.xml')
      .then(res => {
        const xml = XMLMapping.load(res.data);
        const json = XMLMapping.tojson(xml);
        this.setState({ guests: json.dataset.record });
      });
  }

  render() {
    const search = this.state.search.toLowerCase();
    const guests = this.state.guests.filter(guest => (
      guest.first_name.$t.toLowerCase().includes(search) ||
        guest.last_name.$t.toLowerCase().includes(search) ||
        guest.company.$t.toLowerCase().includes(search)
    ));

    return (

        {guests.map((guest, index) => (

            <Guest first_name={guest.first_name.$t} last_name={guest.last_name.$t} company={guest.company.$t} />

{guest.company.$t}

              <EmailFormdisplay guestid={index} />

              <PhoneFormdisplay guestid={index} />

            <GuestDropdown index={index} />

        ))}

    );
  }
}

const Guest = ({ first_name, last_name, company }) => (

    {first_name} {last_name}

{company}

);

const GuestDropdown = ({ index }) => (

    <Dropdown>
      <DropdownTrigger />
      <DropdownContent>

            <EmailFormdisplay guestid={index} />

            <PhoneFormdisplay guestid={index} />

      </DropdownContent>
    </Dropdown>

);

Hope that helps :)


Find the Hardy–Ramanujan number using R5RS scheme I remember once going to see [Srinivasa Ramanujan] when he was ill at Putney. I had ridden in taxi cab number 1729 and remarked that the number seemed to me rather a dull one, and that I hoped it was not an unfavorable omen. "No," he replied, "it is a very interesting number; it is the smallest number expressible as the sum of two cubes in two different ways." [G. H. Hardy as told in "1729 (number)"]

In "Math Wrath" Joseph Tartakovsky says about this feat:

  So what? Give me two minutes and my calculator watch, and I'll do the same without exerting any little gray cells.

I don't know how Mr. Tartakovsky would accomplish that proof on a calculator watch, but the following is my scheme function that enumerates numbers starting at 1 and stops when it finds a number that is expressable in two seperate ways by summing the cubes of two positive numbers. And it indeeds returns 1729.

There are two areas where I would appreciate suggestions for improvement. One area is, being new to scheme, style and idiom. The other area is around the calculations. Sisc does not return exact numbers for roots, even when they could be.

For example (expt 27 1/3) yields 2.9999999999999996. But I do get exact results when cubing an exact number, (expt 3 3) yields 27. My solution was to get the exact floor of a cube root and then test against the cube of the floor and the cube of the floor plus one, counting as a match if either match. This solution seems messy and hard to reason about. Is there a more straightforward way?

; Find the Hardy-Ramanujan number, which is the smallest positive
; integer that is the sum of the cubes of two positivie integers in
; two seperate ways.
(define (hardy-ramanujan-number)
  (let ((how-many-sum-of-2-positive-cubes
          ; while i^3 + 1 < n/1
          ;     tmp := exact_floor(cube-root(n - i^3))
          ;     if n = i^3 + tmp^3 or n = i^3 + (tmp + 1) ^3 then count := count + 1
          ; return count
          (lambda (n)
            (let ((cube (lambda (n) (expt n 3)))
                  (cube-root (lambda (n) (inexact->exact (expt n 1/3)))))
              (let iter ((i 1) (count 0))
                (if (> (+ (expt i 3) 1) (/ n 2))
                    count
                    (let* ((cube-i (cube i))
                           (tmp (floor (cube-root (- n cube-i)))))
                      (iter (+ i 1)
                        (+ count
                          (if (or (= n (+ cube-i (cube tmp)))
                                  (= n (+ cube-i (cube (+ tmp 1)))))
                              1
                              0))))))))))
    (let iter ((n 1))
      (if (= (how-many-sum-of-2-positive-cubes n) 2)
          n
          (iter (+ 1 n))))))

Your code looks mostly fine, I see a few very minor things to comment on:

There's no need to define cube and cube-root at the innermost scope,
Using define for internal functions makes it look a little clearer,
This is related to the second part of your question: you're using inexact->exact on a floating point number which can lead to large rationals (in the sense that you allocate a pair of two big integers) -- it would be better to avoid this,
Doing that still doesn't solve the extra test that you do -- but that's only because you're not certain if you have the right number of if you missed by 1.  Given that it should be close to an integer, you can just use round and then do one check, saving you one test.

Fixing the above, and doing it in one function that returns the number when it's found, and using some more "obvious" identifier names, I get this:

(define (hardy-ramanujan-number n)
  (define (cube n) (expt n 3))
  (define (cube-root n) (inexact->exact (round (expt n 1/3))))
  (let iter ([i 1] [count 0])
    (if (> (+ (cube i) 1) (/ n 2))
      (hardy-ramanujan-number (+ n 1))
      (let* ([i^3   (cube i)]
             [j^3   (cube (cube-root (- n i^3)))]
             [count (if (= n (+ i^3 j^3)) (+ count 1) count)])
        (if (= count 2) n (iter (+ i 1) count))))))

I'm running this on Racket, and it looks like it's about 10 times faster (50ms vs 5ms).


Duplicate entry and rename check I wrote a program that prompts users to enter one string per line and stores it into an array. It also checks for duplicate entries as data are fed and renames those to become distinct by appending an incrementing number. Imagine that it will be used by a large number of people in some name registration event. I am concerned about how slow it will run because of all the comparisons as more and more strings are put.

About the code below, the first line scans how many times inputs will be scanned. I have yet to finish the input validation part as I noticed the above performance issue halfway through writing it.

How can I reduce the number of comparisons and make the program run faster?

import java.util.*;

public class Temp {
    public static void main (String[] args){
        Scanner scanner = new Scanner(System.in);
        int n = scanner.nextInt();
        String[] names = new String[n];
        int namesPoint = 0;
        for(int i=0;i<n;i++) {
            String name = scanner.next();
            int k=0;
            boolean dupe = false;
            for(int j=0;j<namesPoint;j++) {
                if(names[j].equals(name)) {
                    dupe=true;
                    break;
                }
            }
            if(dupe) {
                String newname="";
                while(dupe){
                    dupe=false;
                    k++;
                    StringBuilder sb = new StringBuilder();
                    sb.append(name);
                    sb.append(k);
                    newname = sb.toString();
                    for(int j=0;j<namesPoint;j++) {
                        if(names[j].equals(newname)) {
                            dupe=true;
                            break;
                        }
                    }
                }
                System.out.printf("%s\n",newname);
                names[namesPoint]=newname;
                namesPoint++;
                k=0;
            }else {
                names[namesPoint]=name;
                namesPoint++;
                System.out.printf("OK\n");
            }
        }
        scanner.close();
    }
}

You have the right instinct / deduction that this takes longer than it needs to. This is because each check for duplicates does a 'full scan' over your data. To improve this, you need a way to store / sort your data so that you can quickly ask: "Does this entry already exist?"

Enter the HashSet. A hash set is a data structure that allows for very fast lookups. It does this by mapping entries to an integer (the hash) and then using that to quickly get the right index in an array.

Using a hash set, your program will perform better and have fewer lines of code. The following example code uses the Set.add function that returns true on success (element was added) and false on failure (element was not added, because it already exists).

int n = scanner.nextInt();
Set<String> names = new HashSet<>(n); // expected capacity
while ( n-- > 0 ) {
  String name = scanner.next();
  if ( !names.add(name) ) { // returns false if already present
    // very short but a bit opaque:
    for ( int suffix = 0; !names.add(name + suffix); suffix++ );

    // alternatively, written out:
    for ( int suffix = 0; ; suffix++ ) {
      if ( names.add(name + suffix) ) {
        break;
      }
    }
  }
}

  for ( int suffix = 0; !names.add(name + suffix); suffix++ ); still scans from 0 every time till an unclaimed name is found... Is there perhaps a quicker way?

Indeed, there is—good catch! We can speed up the duplicate counting by using a Map instead, mapping a name to the number of times we've found it. (This is essentially a multi-set, but our standard libaries don't carry such a data structure, so we 'fake' it.)

Our printing code will look quite different, though:

int n = scanner.nextInt();
Map<String, Integer> names = new HashMap<>(n); // expected capacity
while ( n-- > 0 ) {
  String name = scanner.next();
  int count = names.getOrDefault(name, 0);
  names.put(name, count + 1);
  // alternatively:
  // names.merge(name, 1, Integer::sum);
}

// print results
for ( Map.Entry<String, Integer> entry : names.entrySet() ) {
  String name = entry.getKey();
  int count = entry.getValue();
  System.out.println(name); // plain name
  for ( int i = 0; i < count - 1; i++ ) {
    System.out.println(name + i); // number from 0
  }
}

If you're wondering about the performance of HashMap : it is the same as HashSet. In fact, HashSet uses a HashMap under the hood.


A simple semaphore in Java It is a simple semaphore in Java. Although it passes all their tests with flying colors, I'm still not sure it's correct.

For example, the last thing I wasn't sure about was mLock.lockInterruptibly(); vs mLock.lock(); in the method called aquire(). And I ended up using lockInterruptibly() because if the thread calling lockInterruptibly() is interrupted, then InterruptedException is thrown.

package edu.vuum.mocca;

import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.Condition;

/**
 * @class SimpleSemaphore
 *
 * @brief This class provides a simple counting semaphore
 *        implementation using Java a ReentrantLock and a
 *        ConditionObject (which is accessed via a Condition). It must
 *        implement both "Fair" and "NonFair" semaphore semantics,
 *        just liked Java Semaphores.
 */
public class SimpleSemaphore {
    /**
     * Define a ReentrantLock to protect the critical section.
     */
    // TODO - you fill in here
    ReentrantLock mLock;

    /**
     * Define a Condition that waits while the number of permits is 0.
     */
    // TODO - you fill in here
    Condition mCondition;

    /**
     * Define a count of the number of available permits.
     */
    // TODO - you fill in here.  Make sure that this data member will
    // ensure its values aren't cached by multiple Threads..
    volatile int mAvailable;

    public SimpleSemaphore(int permits, boolean fair) {
        // TODO - you fill in here to initialize the SimpleSemaphore,
        // making sure to allow both fair and non-fair Semaphore
        // semantics.
        mAvailable = permits;
        mLock = new ReentrantLock(fair);
        mCondition = mLock.newCondition();
    }

    /**
     * Acquire one permit from the semaphore in a manner that can be
     * interrupted.
     */
    public void acquire() throws InterruptedException {
        // TODO - you fill in here.
        try {
            mLock.lockInterruptibly();
            while (mAvailable == 0) {
                mCondition.await();
            }
            mAvailable--;
            mCondition.signalAll();
        } finally {
            mLock.unlock();
        }
    }

    /**
     * Acquire one permit from the semaphore in a manner that cannot be
     * interrupted.
     */
    public void acquireUninterruptibly() {
        // TODO - you fill in here.
        try {
            mLock.lock();
            while (mAvailable == 0) {
                mCondition.awaitUninterruptibly();
            }
            mAvailable--;
            mCondition.signalAll();
        } finally {
            mLock.unlock();
        }
    }

    /**
     * Return one permit to the semaphore.
     */
    void release() {
        // TODO - you fill in here.
        try {
            mLock.lock();
            mAvailable++;
            mCondition.signalAll();
        } finally {
            mLock.unlock();
        }
    }

    /**
     * Return the number of permits available.
     */
    public int availablePermits() {
        // TODO - you fill in here by changing null to the appropriate
        // return value.
        return mAvailable;
    }
}

You should put the lock statements before the try{}finally.

For example if the thread is interrupted before your interruptible acquire then lockInterruptibly will throw and the finally will try to unlock a non-locked lock (which will throw a IllegalMonitorStateException).

There is no need for the signalAll in the acquire methods, as nothing happens that may require a waiting thread to awake (all that were waiting will still need to wait).

Similarly you only need a signal() call in the release to signal a single waiting thread to awake.

Only the first issue is a true bug the other 2 are performance issues (and invalidate the fairness)


Java Implementation of Quick Sort This is my implementation of quicksort (algorithm taken from Cormen book). This is an in place implementation. Please let us know issues with this or any ideas to make it better. It performs at logN.

import java.util.ArrayList;

public class MyQuickSort {

    /**
     * @param args
     */
    public static void main(String[] args) {

        //int[] a = { 1, 23, 45, 2, 8, 134, 9, 4, 2000 };
        int a[]={23,44,1,2009,2,88,123,7,999,1040,88};
        quickSort(a, 0, a.length - 1);
        System.out.println(a);
        ArrayList al = new ArrayList();
    }

    public static void quickSort(int[] a, int p, int r)
    {
        if(p<r)
        {
            int q=partition(a,p,r);
            quickSort(a,p,q);
            quickSort(a,q+1,r);
        }
    }

    private static int partition(int[] a, int p, int r) {

        int x = a[p];
        int i = p-1 ;
        int j = r+1 ;

        while (true) {
            i++;
            while ( i< r && a[i] < x)
                i++;
            j--;
            while (j>p && a[j] > x)
                j--;

            if (i < j)
                swap(a, i, j);
            else
                return j;
        }
    }

    private static void swap(int[] a, int i, int j) {
        // TODO Auto-generated method stub
        int temp = a[i];
        a[i] = a[j];
        a[j] = temp;
    }
}

If the algorithm was taken from a book, then chances are it will be as good as it could possibly be.  So as long as you followed it to the letter, there really shouldn't be any issues in your implementation.

There is however one thing I think could be improved upon, the interface to initiate the sort.  When I think of sorting a collection, I'd expect to provide a couple of things, the collection of course and possibly a comparer.  Anything else that requires passing implementation specific values just feels "wrong" to me.  It might be ok if these indices represented the start and stop range of values to sort, but I would still make that as a separate overload.

Your implementation on the other hand requires the collection and indices necessary for the algorithm to work.  This would not be an ideal interface to work with since you have to remember to pass in certain values to perform the sort when instead they could be calculated for me.  I would expose an overload which only accepts the collection to sort which would call the actual implementation with the right arguments.

Also, even though this is the well known quick sort algorithm, I'd still provide better variable names.  Not really a problem here, personal preference.

// this overload is the public interface to do the sort
public static void quickSort(int[] collection)
{
    quickSort(collection, 0, collection.length - 1);
}

// note: method is now private
private static void quickSort(int[] collection, int pivotIndex, int rangeIndex)
{
    // etc...
}


Angular directive for unique username I looked at a bunch of AngularJS directives checking for uniqueness of a username and decided to try for the simplest implementation. It does what I want but may not really be 'idiomatic' Angular.

Here is the form element:

<input type="text"
    name="username"
    ng-model="form.username"
    unique-username=""
    required
/>
Username taken!

Here's the directive:

.directive('uniqueUsername', function($http) {
      return {
           restrict: 'A',
           require: 'ngModel',
           link: function (scope, element, attrs, ngModel) {
                element.bind('blur', function (e) {
                     ngModel.$setValidity('unique', true);

                     $http.get("/api/checkUnique/" + element.val()).success(function(data) {
                          if (data) {
                              ngModel.$setValidity('unique', false);
                          }
                     });
                });
           }
      };
})

And the ExpressJS call:

if (data) {
      console.log("found " + data.username);
      return res.send(data.username);
}
else {
      console.log("not found");
      return res.send(404);
}

I would appreciate any feedback on why this is good or bad and if possible a revision that uses $scope.watch on the model.

Here's my two cents. These are the changes that I would make:

Separate the API calls into a service.
Separate the event triggers using the model-options in the view.
Handle the monitoring using a watch. (Expensive, but it seems to be the "angular" way.)
Isolate your code into functions/files, then configure in application.js.

Helper Functions

Normally, I use !!value to test if a value is not undefined or null, but in this case I want to do things properly and make the code readable. Alternatively, if you use library like lodash or underscore, you can just call !_.isEmpty(value)

function notEmpty(value) {
    return (angular.isDefined(value) && !angular.isNull(value))
}

View

Using ngModelOptions will allow one to fine tune the view without having to manipulate the directive. One can then test different events without having to make changes in the directive.

<input type="text"
    name="username"
    ng-model="form.username"
    ng-model-options="{ updateOn: 'blur' }"
    unique-username=""
    required
/>
Username taken!

Service

Using a service is another large advantage. Currently, the directive requires $http and thus becomes bound to the api. Using a service, one can create a mock service to test the directive independently of the api.

Another advantage is that the logic for the uniqueness test is now isolated from the view. This code can then be reused in a controller, another directive, or even another service.

function UserService($q, $http) {

    this.isUnique = function(username) {
        if (notEmpty(username)) {
            var uri = '/api/checkUnique/' + username;
            return $http.get(uri);
        }
        return $q.reject("Invalid User name");
    }

}

Directive

userService is must now be injected into the directive instead of $http . Since the uniqueness test is done when the model changes, one can also create a watch on the model rather than binding it to an event.

In the future, you can use the controller.$parsers.unshift command instead of a scope.$watch. Unfortunately, $parsers doesn't allow for responses from a $promise. See issue #6416.

function UniqueUsernameDirective(userService) {
      return {
           restrict: 'A',
           require: 'ngModel',
           link: function (scope, element, attrs, ngModel) {
               scope.$watch(attrs.ngModel, function(value) {
                   userService.isUnique(value)
                              .then(function(data) {
                                   ngModel.$setValidity('unique', notEmpty(data));
                              })
                              .catch(function() {
                                   ngModel.$setValidity('unique', false);
                              });
               });
           }
      };
}

Application.js

This just keeps your application.js file nice an clean.

angular.module('app', [])
       .service('userService', ['$q', '$http', UserService])
       .directive('uniqueUsername', ['userService', UniqueUsernameDirective]);

Demo

I made a demo using a mock service to demonstrate this functionality. jsFiddle


Balanced parenthesis, with backslash escaping Given a Balanced Parenthesis problem.

  "Balanced Parenthesis

  Create a program that checks if in a given string expression all the parenthesis are balanced.

  For Example:

  (test) - valid

  (no() - invalid

  ()(()) - valid

  (123(456)(7))( - invalid

  (val()id) - valid

  Also, take into account the "\" escape sequences:

  (nope\) - invalid

  (v\(al) - valid

  Too easy? Try doing the same for [] and {} and <>.

  (The description from Sololearn application)"

This is my solution. Do you have any advice? Is it clear enough?

import java.util.Stack;

public class BalancedParenthesis {

private static final Stack<Character> STACK = new Stack<>();
private static final String BRACELETS = "({[<)}]>";
private static final int SEPARATOR = BRACELETS.length() / 2;
private static final char BACKSLASH = '\\';

public static boolean isBalanced(String str) {
    int index; char ch;
    STACK.clear();
    for (int pos = 0; pos < str.length(); pos++) {
        ch = str.charAt(pos);
        if (ch == BACKSLASH) {
            pos ++;
            continue ; }
        index = BRACELETS.indexOf(ch);
        if (isBaceletFind(index)) {
            if (isOpenBracelet(index)) STACK.push(ch);
            else if (isPopable(index)) STACK.pop();
            else return false; }
    }
    return STACK.empty();
}

private static boolean isPopable(int index) {
    return !STACK.empty() && STACK.peek() == BRACELETS.charAt(index - SEPARATOR);
}

private static boolean isBaceletFind(int index) {
    return index != -1;
}

private static boolean isOpenBracelet(int index) {
    return index < SEPARATOR;
}

public static void checkBalance(String str) {
    System.out.print("\"" + str + "\" is");
    System.out.println((isBalanced(str) ? "" : " not") + " balanced");

}
public static void main(String[] args) {
    checkBalance("");
    checkBalance("\\");
    checkBalance("\\(");
    checkBalance("(({[<>]}))");
    checkBalance("))");
    checkBalance("(({[<(>]}))");
    checkBalance("(({[<\\(>]}))");
}
}

Superficial remarks

Whats a "bacelet", in isBaceletFind()?  Did you mean "bracelet"?  Even that is questionable English terminology, as I would take it to mean a piece of jewelry worn at the wrist.  Or maybe you meant "bracket"?

isPopable() should be isPoppable().

I normally don't like to be pedantic about brace styles, but the one you use for your if statements is really very unconventional, and certainly not easy to read.  Wikipedia lists 8 common brace styles, and yours is not any of them.  In fact, I'd say that it's the first time I've ever seen it in use.

Data structure

java.util.Stack is deprecated, because it has bad object-oriented design (it allows random access because it extends java.util.Vector), and because its methods are synchronized (which incurs unnecessary performance overhead).  Normally, the recommended replacement for Stack is ArrayDeque.  However, in this case, since each element is a char, I'd use a StringBuilder instead, so that you don't have to incur the char ↔ Character boxing/unboxing overhead.

Even if your code works, it's a bad practice to write:

private static final Stack<Character> STACK = new Stack<>();

static means that there is only one instance shared throughout the Java interpreter.  Having to write STACK.clear() at the beginning of isBalanced() is ugly.  It also unnecessarily introduces the possibility of multithreading problems, when isBalanced() should be a simple deterministic function with no threading issues.
The ALL_CAPS name suggests that it's a constant, which it isn't in nature, even if it's technically a final variable.

Instead, isBalanced() should instantiate the stack as a local variable, and explicitly pass it to any helper function that needs it.

Algorithm

I'm not a big fan of the BRACELETS constants, with the first half being the opening delimiters, with their counterparts in the second half.  I think it's more cryptic than it needs to be.  I'd just write a switch containing all of the characters of interest: it compiles to very clean and efficient bytecode, even simpler than String.indexOf().

When you encounter an opening delimiter, it would be smarter to push its counterpart onto the stack instead.  That way, you don't have to repeatedly ask "is this the right closing delimiter?" when looking at every subsequent character.

public class BalancedParenthesis {
    private static class CharStack {
        private StringBuilder sb = new StringBuilder();
        public boolean isEmpty() { return this.sb.length() == 0; }
        public void push(char c) { this.sb.append(c); }
        public char pop() {
            char last = this.sb.charAt(this.sb.length() - 1);
            this.sb.setLength(this.sb.length() - 1);
            return last;
        }
    }

    public static boolean isBalanced(String str) {
        CharStack stack = new CharStack();
        for (int pos = 0; pos < str.length(); pos++) {
            char ch = str.charAt(pos);
            switch (ch) {
              // Backslash: ignore the next character
              case '\\': pos++; continue;

              // Opening delimiters
              case '(':  stack.push(')'); break;
              case '{':  stack.push('}'); break;
              case '[':  stack.push(']'); break;
              case '<':  stack.push('>'); break;

              // Closing delimiters
              case ')':
              case '}':
              case ']':
              case '>':
                if (stack.isEmpty() || stack.pop() != ch) {
                    return false;
                }
            }
        }
        return stack.isEmpty();
    }
}


Matrix Multiplication Python -- Memory Hungry Here is a snippet from my python script where I am performing:

a dictionary lookup
a cartesian multiplication of a list of len=500 against a list of len=60,
calculating a cumulative addition for each multiplcation combination
repeat this down a 20,000 rows

This is slowing things way down and making it hard to debug with the ~10 min wait times.  How can I write this in a more efficient way, and that makes use of more than just one CPU?

ccle_head, dot_prod_total = [], []
with open(ccle_mRNA_file, mode='r') as ccle:
    for _ in range(2): #skip to data
        next(ccle)
    exp_mat = [line.rstrip().split('\t') for line in ccle]
for row in exp_mat:
    if row[0] != 'Name' and row[1] in pat_mRNA: # data row
        n_row = [float(s) for s in row[2:]]
        # get ready for some slow matrix multiplication:
        prod = [p*n for n in n_row for p in pat_mRNA[row[1]]] # slows here
        if dot_prod_total:
            dot_prod_total = [cum_prod + prod for (cum_prod,prod)
                        in zip(dot_prod_total,prod)] # cumulative addition
        else: dot_prod_total = prod  # first record of matrix products
     else: ccle_exp_head = row   # header row

Here are the docs for the input params:

"""
:param ccle_mRNA_file: text file, rows are gene expression across sample columns
:param pat_head: list, patient id [pat_1(str), pat_2...]
:param pat_mRNA: dict, patient RNA {gene(str):[pat_1(float), pat_2..}
:param cell_interest: set, cells of same tissue {cell_1, cell_2}
"""

I'm not sure I completely follow exactly what processing you're doing to your data. The comments in the code suggest it's meant to be a matrix multiplication, the description you've provided above it suggests it's a cartesian product, but your code looks like does the sum of (flattened) outer products of corresponding rows. I'll proceed on the assumption that your code is doing the right thing, but hopefully you can clarify.

There are two general pieces of advice that apply here:

Separation of concerns. You're mingling code to parse your data with code to process it once it is parsed. Instead, parse everything into the appropriate data structures first, and then do the processing.
Use appropriate libraries and data structures. When you're doing linear algebra, that means numpy. Numpy is a dedicated, LA library that has heavily optimised routines for things like various matrix products, so you don't need to roll your own. Numpy arrays will also use quite a bit less memory than Python lists. However, what you're working with is labelled data (and some of it is labelled in two dimensions), which means the best data structure is probably a pandas dataframe - it's basically like your dict of strings to lists in pat_mRNA, but much more optimised, and fully compatible with numpy's matrix manipulations.

These things together will make your code easier to follow, more maintainable, and often both faster and shorter.

So, start by looking at your core data structure:

exp_mat = [line.rstrip().split('\t') for line in ccle]

This is a list of lists of strings. For most of the rows, you don't seem to care about column 0 at all. Column 1 contains row labels, and corresponding to keys in your dictionary pat_mRNA. The rest of it is numeric data, currently stored as strings. It also contains 'header' rows - you only keep one of those around, so presumably you only expect one to be in there. I'm guessing that is column labels, which a DataFrame lets you include directly in the same data structure.

The only trick parsing this into a DataFrame is that it is a little easier (and, apparently, more efficient) to swap the columns and rows from how you have them currently.  That will affect how it prints, and which methods you call to relabel things, but not much else. This is how I would parse it into a DataFrame:

data = pd.DataFrame()
headers = []
for line in ccle:
    row = line.rstrip().split('\t')
    if row[0] == 'Name':
         headers = row
    elif row[1] in pat_mRNA:
        data[row[1]] = [float(f) for f in row[2:]]

if headers:
   data.index = headers

Note that I have modified your conditions a little, since yours seemed slightly off. The else branch of your condition effectively says "if the zeroth column is "name" or the first column isn't a match, it must be a header" - I've changed it so that instead, an appropriate value in the zeroth column is a header, and a non-matching first column on is ignored.

I've maintained a temporary 'headers' list, because your code is currently robust against those appearing anywhere in the file. A better way to maintain that robustness would be if you know - or, preferably, can parse from your file before now - how many variables you have data for. Then you could do away with that headers list and do this:

data = pd.DataFrame(index=range(nvars))
for line in ccle:
    row = line.rstrip().split('\t')
    if row[0] == 'Name':
         data.index = row
    elif row[1] in pat_mRNA:
        data[row[1]] = [float(f) for f in row[2:]]

In either case, I am assuming that you have one header for each variable. If that's a bad assumption, you may need to adjust this code.

As an aside, as a matter of commonly-accepted Python style (and in accordance with PEP 8), I have placed all of the code for each branch of the if in indented blocks, even if it is only one line. At most, you should only put it immediately after the : if there is only one branch to the if (no else or elifs), and even then, consider an indented block anyway.

I would change pat_mRNA to also be a DataFrame. You could change how you currently build it, or you could just pass the whole dictionary you have at the moment into pd.DataFrame().

Once you've done that, you can use numpy to do your LA work. As written, your entire strange product becomes this:

dot_prod_total = sum(np.outer(data[key], pat_mRNA[key]).flat for key in data)

Which should be quite a lot faster than trying to do it by list comprehensions, and may use multithreading depending on how numpy is compiled.


Callback functions to select the tab to the left in Google Chrome I'm a JavaScript rookie, but I wrote a JavaScript function as part of a Chrome extension that will activate the tab to the left of the currently active tab. The function is pretty ugly, but I couldn't figure out another way to write it.

The logic is pretty simple, but I ran into problems setting variables throughout the various scopes. How would I move the functionality of the Chrome API calls into individual functions that return values? Can callback functions return a value?

// Activates the tab to the left of the currently active tab
tabLeft = function() {
    var parent = this;
    var activeTabIndex;
    var targetTabIndex;

    // Get the index of the active tab
    chrome.tabs.query({'active':true, 'currentWindow': true}, function(tabs) {
        parent.activeTabIndex = tabs[0].index;
    });

    // Get the count of the number of tabs in the current window
    chrome.tabs.query({'currentWindow': true}, function(tabs) {
        var tabCount = tabs.length;
        parent.targetTabIndex = (parent.activeTabIndex == 0) ? (tabCount - 1) : (parent.activeTabIndex - 1);

        // Get the tabId of the of the tab we want to activate
        chrome.tabs.query({'index': parent.targetTabIndex}, function(tab) {
            var tabId = tab[0].id;

            // Activate the target tab
            chrome.tabs.update(tabId, {'active': true}, function() {});
        });
    });
};

Some of the Chrome APIs are implemented using callbacks. This means that these methods are potentially async. Code doesn't run in the order they're written, you have been warned.

The closest you'll get to sequentially written async code is by using Promises. In simple terms, these are objects that hold some state, and when a certain state is attained (resolve or reject), the attached handlers get executed.

The beauty lies in then which when returned a promise, attaches the next chained then to that promise instead. This makes the next then appear to wait for the previous async operation. However, when returned a non-promise value, it uses that value as the resolved value for the next chained then. The ability make the code appear to wait, the ability to alter the resolved values as well as carry values through makes Promises powerful.

I think Chrome should have support for Promises already. Here's an example where a we wrap the chrome.tabs.query in a promise, and when it succeeds, it resolves the promise.

function promiseQuery(options){
  return new Promise(function(resolve,reject){
    chrome.tabs.query(options, resolve);
  });
}

function promiseUpdate(tabId,options){
  return new Promise(function(resolve,reject){
    chrome.tabs.update(tabId,options,resolve);
  });
}

function tabLeft(){

  promiseQuery({'active':true, 'currentWindow': true})
  .then(function(tabs){
    // This function executes when the promiseQuery resolves

    // Get the current index
    var activeIndex = tabs[0].index;

    // We fire another query. But instead of returning a promise, we can chain a then
    // to alter the resolved value.
    return promiseQuery({'currentWindow': true})
      .then(function(tabs){
        return{
          length : tabs.length,
          activeIndex : activeIndex
        }
      });
  })
  .then(function(data){
    // This executes with data being the altered resolved value

    var targetIndex = ((data.activeIndex === 0) ? data.length : data.activeIndex) - 1;
    return promiseQuery({'index': targetIndex});
  })
  .then(function(tabs){
    // same here
    var tabId = tab[0].id;

    // As long as you return a promise
    return promiseUpdate(tabId, {'active': true});
  })
  .then(function(){
    // Done
  });
}

For now, it still looks like a jumble. However, you can now pull out functions and just hand references to then. There's no more problem with persisting values in an outer scope since you can just pass them through the then chain.

function promiseQuery(options){
  return new Promise(function(resolve,reject){
    chrome.tabs.query(options, resolve);
  });
}

function promiseUpdate(tabId,options){
  return new Promise(function(resolve,reject){
    chrome.tabs.update(tabId,options,resolve);
  });
}

function initialCall(){
  return promiseQuery({'active':true, 'currentWindow': true})
}

function activeAndLengthQueries(){
  var activeIndex = tabs[0].index;
  return promiseQuery({'currentWindow': true})
    .then(function(tabs){
      return{
        length : tabs.length,
        activeIndex : activeIndex
      }
    });
}

function targetTabQuery(){
  var targetIndex = ((data.activeIndex === 0) ? data.length : data.activeIndex) - 1;
  return promiseQuery({'index': targetIndex});
}

function tabUpdateFunction(){
  var tabId = tab[0].id;
  return promiseUpdate(tabId, {'active': true});
}

function doneFunction(){

}

function tabLeft(){
  initialCall()
  .then(activeAndLengthQueries)
  .then(targetTabQuery)
  .then(tabUpdateFunction)
  .then(doneFunction);
}

Breaking down an operation into promises has no fixed rule. However, I write down the code in the sequence I want (just a flat list of things to do). Then run though them again. This time, if you see an async operation, that ends a then (notice how the previous example ends with return somePromiseGeneratingFunction). Do that operation and return it's promise. The sequence will continue in the next then.


Large number stored as vector, increase performance I made a code to initialize char vectors with random numbers representing very large numbers with more than 20.000 digits. The additions and multiplications are correct but my code is too slow (1 minute). I know that I can change char by 64 int to make operations with 19 numbers at a time, instead of 1 digit at a time, but I don't know how to do that without changing my current output that is correct.

Thanks for your help

#include <stdio.h>
#include <stdlib.h>

unsigned int seed;

int newrandom() {
  seed = (214013*seed+2531011);
  return (seed>>13);
}

void LongNumInit( char *L, unsigned N )
{
  for ( int i=0; i< N; i++ )
  {
    L[i] = newrandom() % 10;
  }
}

void PrintNumber( char *L, unsigned N, char *Name )
{
  printf("%s:", Name);
  for ( int i=N; i>0; i-- )
  {
    printf("%d", L[i-1]);
  }
  printf("\n");
}

void ValueSet( char *L, unsigned N, char digit )
{
  for ( int i=0; i< N; i++ )
  {
    L[i] = digit;
  }
}

void CPNumber( char *Vin, char *Vout, unsigned N )
{
  for ( int i=0; i< N; i++ )
  {
    Vout[i] = Vin[i];
  }
}

char Add( char *Vin1, char *Vin2, char *Vout, unsigned N )
{
  char CARRY = 0;
  for ( int i=0; i< N; i++ )
  {
    char R = Vin1[i] + Vin2[i] + CARRY;
    if ( R <= 9 )
    {
      Vout[i] = R; CARRY = 0;
    }
    else
    {
      Vout[i] = R-10; CARRY = 1;
    }
  }
  return CARRY;
}

char DigitAddition( char *V, char digit, unsigned N )
{
  int i=0;
  char R = V[0] + digit;
  if (R < 10)
  {
    V[0] = R; return 0;
  }

  V[0] = R-10;
  char CARRY = 1;
  i = 1;
  while ( CARRY && i < N )
  {
    if ( V[i] < 9 )
    {
      V[i] = V[i] + 1; CARRY = 0;
    }
    else
    {
      V[i] = 0; i++;
    }
  }
  return CARRY;
}

char AddInHorizontal( char *Vin, char *Vout, unsigned N )
{
  char CARRY = 0;
  ValueSet ( Vout, N, 0 );
  for ( int i=0; i< N; i++ )
  {
    DigitAddition ( Vout, Vin[i], N );
  }
  return 0;
}

char MultiplyConst( char *V, unsigned N, char digit )
{
  char CARRY = 0;
  for ( int i=0; i< N; i++ )
  {
    char R = V[i] * digit + CARRY;
    CARRY = R / 10;
    R = R - CARRY*10;
    V[i] = R;
  }
  return CARRY; // may be from 0 to 9
}

void Mult( char *Vin1, char *Vin2, char *VoutH, char *VoutL,  unsigned N )
{

  unsigned char *TEMP= (unsigned char*) malloc( 2*N*sizeof(unsigned char) );
  unsigned char *RES = (unsigned char*) malloc( 2*N*sizeof(unsigned char) );

  ValueSet  ( RES, 2*N, 0 );    // Set RES to 0

  for ( int i=0; i<N; i++ )
  {
    ValueSet  ( TEMP, 2*N, 0 );
    CPNumber ( Vin1, TEMP+i, N );
    MultiplyConst( TEMP, 2*N, Vin2[i] );
    Add ( TEMP, RES, RES, 2*N ); // TEMP + RES -> RES
  }

  CPNumber ( RES,   VoutL, N );
  CPNumber ( RES+N, VoutH, N );
}

int main (int argc, char **argv)
{
  int i, sum1, sum2, sum3, N=20000;

  seed = 12345;

  if (argc>1) { N    = atoi(argv[1]); }
  if (argc>2) { Rep  = atoi(argv[2]); }

  unsigned char *V1= (unsigned char*) malloc( N*sizeof(unsigned char) );
  unsigned char *V2= (unsigned char*) malloc( N*sizeof(unsigned char) );
  unsigned char *V3= (unsigned char*) malloc( N*sizeof(unsigned char) );
  unsigned char *V4= (unsigned char*) malloc( N*sizeof(unsigned char) );

  LongNumInit ( V1, N );
  LongNumInit ( V2, N );
  LongNumInit ( V3, N );

  Add ( V1, V2, V4, N );
  Mult ( V3, V4, V2, V1, N );
  AddInHorizontal ( V1, V2, N );
  DigitAddition ( V3, V2[0], N );

  PrintNumber( V1, 32, "V1" );
  PrintNumber( V2, 32, "V2" );
  PrintNumber( V3, 32, "V3" );
  PrintNumber( V4, 32, "V4" );

  free(V1);
  free(V2);
  free(V3);
  free(V4);
  return 0;
}

```

Congratulations on having code ready to review!  Here are some thoughts on how you could improve this:

You have tagged this code as both C and C++. To me, this looks purely like C code. If so, please edit the tags. If this needs to work as both C and C++ code, you'll want to specify what other constraints apply.
C code is generally written using snake_case for function names. For a module of functions operating on a shared data type, a short prefix is often used. Since you are calling your objects "LongNum", I suggest you rename your functions to use a prefix of lnum_ or lngn_ or lnm_ or something.

char Add( char *Vin1, char *Vin2, char *Vout, unsigned N ) {...}

becomes

char lnum_add(char * ln1, char *ln2, char *lnout, unsigned N) {...}

Creating a LongNum requires allocating memory. That operation can and should be part of the library by default. (You might want to code a function for "initialize longnum from user-supplied memory" but I doubt you'll need it.) So you should not be doing malloc() or free() calls directly, but instead you should be calling lnum_new_random() and lnum_delete().
I don't see a reason to avoid the C++ naming scheme, so I suggest calling your creation/allocation functions new and your free functions delete.
Related to (4), I believe a good library should be able to create objects in various ways. Particularly, a random value, a fixed value like zero or one, an arbitrary integer value, a value from a double, and possibly values from strings, and/or values from an input stream.

lnum_new_random(unsigned max_digits)
lnum_new_zero(unsigned max_digits)
lnum_new_from_int(unsigned max_digits, long long init_val)
lnum_new_from_unsigned(unsigned max_digits, unsigned long long init_val)
lnum_new_from_double(unsigned max_digits, long double init_val)
lnum_new_from_string(unsigned max_digits, const char * str)
lnum_new_from_file(unsigned max_digits, FILE * input)
lnum_new_from_lnum(LNUM original)

Instead of passing a parameter N around to specify the maximum length of the data type, why not build a struct to store the information.

For starters, you could define a struct with your max_digits value and the pointer to the start of the digits buffer. A more advanced version would eliminate the pointer and allocate the digit buffer immediately after the max_digits value.

struct LNUM {
    unsigned max_digits;
    char * digits;
};

By doing this, you could decide whether passing the objects as pointers or struct values made more sense. So you would want a named type that you control, rather than just letting the user pass around a char * pointer:

typedef struct LNUM LNUM;     // if struct LNUM includes digits as pointer
typedef struct LNUM * LNUM;   // if struct LNUM includes digits as array

Your PrintNumber function provides the only way to extract the number. And it hard-codes the radix, the output stream, and the format. I'd suggest writing a function to emit an LNUM onto a stream, and one to write an LNUM into a string buffer in a provided radix.

size_t lnum_formatted_length(LNUM ln);
void lnum_format_to_string(LNUM ln, char * buffer, size_t max_chars);

Instead of treating N as a constant, would it be so wrong to have N dynamically computed? Your multiplication function returns two numbers, "low" and "high", each of size N. Why not simply return a single number that has the correct size as determined by your multiply function? Maybe it needs more digits, maybe it doesn't.


Most common words in a text file of about 1.1 million words I'm learning programming with Python.

I’ve written the code below for finding the most common words in a text file that has about 1.1 million words. It is working fine, but I believe there is always room for improvement.

When run, the function in the script gets a text file from the command-line argument sys.argv[1], opens the file in read mode, converts the text to lowercase, makes a list of words from the text after removing any whitespaces or empty strings, and stores the list elements as dictionary keys and values in a collections.Counter object. Finally, it returns a dictionary of the most common words and their counts. The words.most_common() method gets its argument from the optional top parameter.

import sys
import collections

def find_most_common_words(textfile, top=10):
    """ Return the most common words in a text file. """

    textfile = open(textfile)
    text = textfile.read().lower()
    textfile.close()
    words = collections.Counter(text.split()) # how often each word appears

    return dict(words.most_common(top))

filename = sys.argv[1]
top_five_words = find_most_common_words(filename, 5)

This is actually quite good. Good use of the collections module.

One improvement I can think of is switching to the with open(...) as: structure, that way the file will automatically be closed when finished processing.
Secondly it is standard in python to use a if __name__ == '__main__': block.
Lastly sys.argv will work ok for programs with not many arguments, but I'd recommend looking at the argparse module.


Implement Java hasNext() and next() in C++ Recently came upon this question asked in interview with major tech company.

  Implement Java's next and hasNext function in your favorite
  programming language.

Here is the solution I came up with. I am not sure whether next() is implemented correctly. Here is the iterator documentation.

#include <iostream>
#include <vector>
#include <string>

template <class T>
class iterator {
public:
    iterator() {}
    iterator(std::vector<T> vec) {
        container = vec;
    }
    bool hasNext() {
        if (curr_pos == container.size()) {
            return false;
        } else {
            if (curr_pos < container.size()) {
                return true;
            }
        }
        return false;
    }
    T next(){
        if (hasNext()) {
            curr_pos += 1;
            return container.at(curr_pos-1);
        }
    }
private:
    std::vector<T> container;
    int curr_pos = 0;
};

int main() {
    std::vector<int> int_vec {1,2,3,4};
    iterator<int> int_it(int_vec);
    while (int_it.hasNext()) {
        std::cout << int_it.next() << " ";
    }

    std::cout << std::endl;

    std::vector<std::string> str_vec {"a", "b", "c","d"};
    iterator<std::string> str_it(str_vec);
    while(str_it.hasNext()) {
        std::cout << str_it.next() << " ";
    }
    std::cout << std::endl;
    std::vector<int> empty_vec;
    iterator<int> empty_it(empty_vec);
    while(empty_it.hasNext()) {
        std::cout << empty_it.next() << " ";
    }
    return 0;
}

Here is the working version of the above code.

I see some things that may help you improve your code.

Make sure all control paths return

Within your implementation of next(), what happens if hasNext() is not true?  The calling function will be expecting a type T but it won't get anything.  This should be addressed.

Consider throwing an error

The Java version of next() throws a NoSuchElementException if there are no more elements.  You could reproduce this behavior by actually deleting code (which would also address the above issue):

T next(){
    return container.at(curr_pos++);
}

This works because at throws a std::out_of_range error if there are no further items.

Think carefully about signed vs. unsigned

The curr_pos member does not have a reasonable interpretation for negative values, so it would make more sense to declare it unsigned rather than int.

Prefer modern initializers for constructors

The constructor could be written using the more modern parameter intialization style.  That version would look like this:

iterator(std::vector<T> vec)
    : container{vec}, curr_pos{0}
{}

Use const where practical

The current hasNext() routine does not (and should not) modify the underlying object, and so it should be declared const:

bool hasNext() const { /* code */ }

Simplify expressions

The hasNext() routine is overly complex.  It could instead be this single line:

bool hasNext() const {
    return curr_pos < container.size();
}

Use const references where practical

The constructor could and should take a const reference to avoid a copy and to preserve flexibility.  You could also use default parameters to eliminate the no-argument constructor:

iterator(const std::vector<T> &vec = {})
    : container{vec}, curr_pos{0}
{}

Consider providing an initializer list constructor

The code has a few versions of something like this:

std::vector<int> int_vec {1,2,3,4};
iterator<int> int_it(int_vec);

It would be convenient to instead be able to write this:

iterator<int> int_it{1,2,3,4};

Here is a constructor that will allow that usage:

iterator(const std::initializer_list<T>& v)
    : container{v}, curr_pos{0}
{}


Implementation of Bridge Design Pattern I made an implementation of the Bridge Pattern to handle ever-changing in crawler APIs that I'm using in my APP.

public interface CrawlerApi {
    // common constants to serveral implementations.
    String CHARSET = "ISO8859_1";
    int TIMEOUT = 20000;

    /**
     * Visit an url and return contents in String format.
     *
     * @param url
     * @return
     */
    String visit (String url);

    /**
     * Parse an Html code into an Navigable object that is provided by the API.
     *
     * @param htmlCode
     * @param documentType
     * @param <T>
     * @return
     */
    <T> T getDocument (String htmlCode, Class <T> documentType);

}

public class HtmlUnitCrawlerApi implements CrawlerApi {
    private WebClient webClient;

    public HtmlUnitCrawlerApi() {
        this.webClient = createWebClient();
    }

    @Override
    public String visit(String url) {
        Page p = null;
        try {
            p = webClient.getPage(url);
            return p.getWebResponse().getContentAsString(CHARSET);
        } catch (IOException e) {
            return "";
        }
    }

    @Override
    public <T> T getDocument(String htmlCode, Class<T> documentType) {
        throw new IllegalArgumentException("not implemented");
    }

    private WebClient createWebClient() {
        WebClient webClient = new WebClient(BrowserVersion.FIREFOX_38);
        webClient.getOptions().setJavaScriptEnabled(true);
        webClient.getOptions().setCssEnabled(false);
        webClient.getOptions().setThrowExceptionOnScriptError(false);
        webClient.setAjaxController(new NicelyResynchronizingAjaxController());
        webClient.waitForBackgroundJavaScript(TIMEOUT);
        return webClient;
    }

}

public class JsoupCrawlerApi implements CrawlerApi {
    public static final String USER_AGENT = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/33.0.1750.152 Safari/537.36";

    @Override
    public String visit(String url) {
        return connect.apply(url).html();
    }

    //I made this static because of the peculiarity of api that is static and because I have to
    //set some config parameters.
    private static Function<String, Document> connect = url -> {
        try {
            return Jsoup.connect(url).timeout(TIMEOUT).userAgent(USER_AGENT).postDataCharset(CHARSET).get();
        } catch (IOException e) {
            return new Document("");
        }
    };

    private static Function<String, Document> parse = html -> {
        Document doc = Jsoup.parse(html);
        doc.charset(Charset.forName(CHARSET));
        return doc;
    };

    @Override
    public <T> T getDocument(String htmlCode, Class<T> type) {
        return (T) parse.apply(htmlCode);
    }

}

Usage:

public class ApiClientExample {
    private CrawlerApi crawlerApi;

    public ApiClientExample(CrawlerApi crawlerApi) {
        this.crawlerApi = crawlerApi;
    }

    public void visit (String url){
        crawlerApi.visit(url);
    }
}

Concerns:

Is this code clean enough?

Is there anything that can be improved?

I looked at it twice and I do not see any kind of Bridge Pattern. I see a simple abstraction and some concrete implementations of it. "ApiClientExample" is maybe a wrapper. Technically it seems to me ok.

Nevertheless I'd like to comment on one semantic thing:

As the method "visit" is totally fine, the method "getDocument()" seems strange to me. Without totally rejecting it I'd make some research on the liskov substitution principle. You also should consider a wrong abstraction level or a wrong semantical assigment to the interface "CrawlerAPI" as the method "getDocument" is totally generic. Will the return value of "visit" be put into "getDocument()"? If so, the method is at the wrong place. You introduced a mental mapping and it would be more a cascading call than a method delared in the same interface.


Generate consecutive sentence of int values accessed from different threads I need to generate a sequence of int values: { 0, 1, 2, ... }, but there's a twist: I need to access them from different threads.

So I wrote this code:

class Counter
{
    private static Counter instance = new Counter();
    private static int i = 0;

    public static Counter Instance
    {
        get
        {
            return instance;
        }
    }

    public int Next()
    {
        lock (this)
        {
            return i++;
        }
    }

    public void Reset()
    {
        lock (this)
        {
            i = 0;
        }
    }

}

May this be implemented in a simpler way?

private static int i = 0;

Are you sure that i should be static? If it's static there isn't too much sense of the Counter instance = new Counter() instance and the Next and the Reset methods also could be static.

Anyway, I'd use a longer variable name, like nextValue for better readability.

Furthermore, consider the drawbacks of the Singleton pattern:

  This pattern makes unit testing far more difficult,
  as it introduces global state into an application.

More on Wikipedia.


Does this method contain multiple implementations? My teammate and I have a small discussion about the responsibility of a method. He thinks next method has two responsibilities and I think that only one.

 private boolean validateUsername(String username) {
    AccountValidationResult validationResult =
            mAccountValidator.validate(username, new int[]{AccountValidation.VALIDATION_REQUIRED});

    if (!validationResult.isSuccessful()) {
        mView.setUsernameModeError(validationResult.getMessage());
    } else {
        mView.setUsernameModeStandard();
    }
    return validationResult.isSuccessful();
}

I think that the responsibilities of validating the string "username" falls on the "validate" method of my mAccountValidator collaborator. Depending of the result returned by my collaborator, I change the state of the view and finally, in order to finish the process or not, I return the state of validation. This result is managed by the next method:

public void doLogin(UserCredentialModel userCredential) {

    if (!validateUsername(userCredential.getUsername())) {
        return;
    }

    if (!validatePassword(userCredential.getPassword())) {
        return;
    }

    mDoLogin.setUserCredential(userCredential);
    mDoLogin.execute();
}

What do you think about my solution? Do you think that is it enough clean or how I could improve it?

Thank a lot for your time and help.

Seemed fine at first, then I took another look...

The validation function displays the error message?

I'm sorry, but that's indeed a violation of SRP. You'd be better off returning a ValidationResult which you could use. You already HAVE this validation result... so in a way, you've already done what I said.

What's wrong is that your function names do not reflect this. You call validateUsername, but it also displays errors. Rather than changing the validation method to return a ValidationResult (which would imply taking out validateUsername and doing everything in doLogin, you should change the function name instead.


Converting float from arbitrary base to arbitrary base I needed to write a small program to convert floats from any base to any other base. The thing is, it got big. I know there are other ways to implement the conversions (such as doing subtractions instead of divisions), but isn't there a more simple way to do the whole process? I feel like I'm missing something. Any optimization tips are welcome as well.

import string
symbols = string.digits + string.uppercase

def _int_from_base(number, original_base):

    return int(number, original_base)

def _int_to_base(number, new_base):

    # Uses "the division method"
    sign = -1 if number < 0 else 1
    number *= sign
    ans = ''
    while number:
        ans += symbols[number % new_base]
        number //= new_base
    if sign == -1:
        ans += '-'
    return ans[::-1]

def _fractional_from_base(number, original_base):

    # The value of a symbol at position i after the decimal point is, by
    # definition, the value of that symbol * b^-i
    ans = 0
    for i in xrange(1, len(number)+1):
        ans += symbols.index(number[i-1]) * original_base**-i
    return ans

def _fractional_to_base(number, new_base, precision=5):

    # I don't know what this method is called
    ans = ''
    for i in xrange(precision):
        tmp = number * new_base
        itmp = int(tmp)
        ans += str(symbols[itmp])
        number = tmp - itmp
    return ans

def convert(number, original_base, new_base, precision=None):
    """Converts any number from any base to any other base (2 <= base <= 36).

    number should be a string representing a float in any base, e.g. '1.23'.

    original_base, new_base should be integers representing the desired bases.

    precision should be an integer representing how many digits after the
    decimal point will be calculated on the conversion. Default is the same
    number as the number of digits after the decimal point on number.
    """

    try:
        integer_part, fractional_part = number.split('.')
        precision = len(fractional_part) if precision is None else precision
        integer_part = _int_to_base(
            _int_from_base(integer_part, original_base),
            new_base
        )
        fractional_part = _fractional_to_base(
            _fractional_from_base(fractional_part, original_base),
            new_base,
            precision
        )
        return integer_part+'.'+fractional_part
    except ValueError:      # number was a str representing an int not a float
        return _int_to_base(_int_from_base(number, original_base), new_base)

I didn't test it thoroughly but it seems to work. A few test cases:

>>> a = convert('632.6442', 10, 2);a # Loss of info. is expected
'1001111000.1010'
>>> convert(a, 2, 10)
'632.6250'

>>> b = convert('222.0', 3, 16);b
'1A.0'
>>> convert(b, 16, 3)
'222.0'

It is not necessary to deal separately with the integral and fractional parts.

When converting from string, remove the point, convert as integer and scale according to the position where the point was.
When converting to string, scale to integer, convert and insert point in the right place.

Here's how you can implement convert with _int_to_base as the only helper function:

def convert(number, original_base, new_base, precision=None):
    #from original_base
    integral, point, fractional = number.strip().partition('.')
    num = int(integral + fractional, original_base) * original_base ** -len(fractional)

    #to new_base
    precision = len(fractional) if precision is None else precision
    s = _int_to_base(int(round(num / new_base ** -precision)), new_base)
    if precision:
        return s[:-precision] + '.' + s[-precision:]
    else:
        return s


generate a 2d array that could be used to create an image with Java This Python code
im_data = np.ones((100,100))
im_data[20:50,20:50]=np.zeros((30,30))

can generate a 2d array that could be used to create an image
plt.axis('off')
axi = plt.imshow(im_data, cmap='Greys')

I'm trying to do the same job with Java
class Arr100{
    public static void main(String[] args){
        System.out.println("H");
        int[][] arr = new int[100][100];
        for(int i=0; i<arr.length; i++){
            for (int j=0; j<arr[i].length; j++){
                arr[i][j]=1;
            }
        }
        for(int i=20; i<50; i++){
            for (int j=20; j<50; j++){
                arr[i][j]=0;
            }
        }
    }
}

In terms of GPU and memory, is there a better way to do the same thing?

You don't need to manually manage nested for loops.
int[][] pixels = new int[100][100];
for (int[] row : pixels) {
    Arrays.fill(row, 1);
}

for (int i = 20; i < 50; i++) {
    Arrays.fill(pixels[i], 20, 50, 0);
}

This will make better use of optimizations available for managing array values.
I personally don't like arr as a name for an array.  If I need a generic name, I tend to use data.  I used pixels here.  Or colors would work.
This may be more or less efficient than
int[][] pixels = new int[100][100];
int i = 0;
for (; i < 20; i++) {
    Arrays.fill(pixels[i], 1);
}
for (; i < 50; i++) {
    Arrays.fill(pixels[i], 0, 20, 1);
    Arrays.fill(pixels[i], 50, pixels[i].length, 1);
}
for (; i < pixels.length; i++) {
    Arrays.fill(pixels[i], 1);
}

That doesn't set any of the pixels twice and relies on the default values for the 0 pixels.  That may be more efficient.  Although it is also possible that it is easier to set the entire array to one value than to work with rows and parts of rows.
Note that if you are willing to change the representation, then
int[][] pixels = new int[100][100];
for (int i = 20; i < 50; i++) {
    Arrays.fill(pixels[i], 20, 50, 1);
}

would probably be better than either.  In this, black is the default 0 while 1 is the white square.
The third form is the least code and probably fastest.  The only way that I could see it being as slow or slower than either of the others is if something is happening implicitly.  For example, if there is a memory initialization that the first form skips because it immediately does its own initialization.
The first form is less code than the second.  The comparative speed would probably depend on the platform (and possibly compiler).  Platforms that allow for managing memory as blocks would probably find the first quicker, while platforms that can only address one word at a time would probably find the second quicker.
You would have to do timing tests on all platforms where you expect to run to get a real idea of the speed.
There is probably a streams-based version that is faster as well.
If you want to go further and try to pass the information for regions, you would either have to write your own solution or use a third party library.  Note that such a thing would probably be slower.  But it might be less code.  I suspect that your Python example is also slower on some platforms (albeit less code).  My guess is that Python implements it much as the first example code block in this answer.


Filtering a SELECT group using jQuery I want to filter the options of a large select group based on text input in a textbox. I am currently using this code to filter from a SELECT group based on the text of a input box.

Can you please tell me whether there is a more efficient way of doing this? My SELECT group is very large with around 7000 entries, which is why I am looking for the most optimized way.

<script>
var fullcelllist = $();
var lastVal ="";

$(window).load(function(){
    $("#celllist option").each(function(){
        fullcelllist.push($(this).val());
    });
});

$("#filtercell").keyup(function() {
    if(lastVal.length < $(this).val().length){
        $("#celllist option:not([value*='"+$(this).val().toUpperCase()+"'])").each(function() {
            $(this).remove();
        });
    }else{
        stregexp = new RegExp($(this).val(),"i");
        temparr = $.grep(fullcelllist, function (value) {
        if(value.match(stregexp)) return value;
            return null;
        });
        newopt = "";
        temparr.sort();
        $.each(temparr,function(i){
            newopt += "<option value='"+temparr[i]+"'>"+temparr[i]+"</option>";
        });
        $("#celllist").html("");
        $("#celllist").append(newopt);
    }
    lastVal = $(this).val();
});
</script>

<input type="text" id="filtercell">
<select id ="celllist" multiple="multiple">
// HERE all OPTIONS using PHP
<select>

Your jQuery could be somewhat more efficient if you minimize $(...) element look-ups by storing them as variables or chaining them.

Also, I think you will see some benefit to replacing some jQuery constructs with their pure JS alternatives ($.each -> for, etc..)

Check out this set of jsPerf tests from this StackOverflow answer.

<script>
var fullcelllist = $(),
    lastVal ="",
    $celllistOption = $("#celllist option");

// ...

$("#filtercell").keyup(function() {
    var $this = $(this),
        $thisVal = $this.val();

// ...


To print asterisks based on inputs I have to print the number of stars that increases on each line from the minimum number until it reaches the maximum number, and then decreases until it goes back to the minimum number. After printing out the lines of stars, it should also print the total number of stars printed.

I have tried using shell scripting and worked. But is there any other simplified and generic way to achieve this. Or to be precise any better code to achieve this

echo "enter the mininum number of stars"
read min
echo "enter the maximum number of stars"
read max
for (( i=$min;i<=$max;i++))
do
   for (( j=$max;j>=i;j-- ))
   do
   echo -n " "
   done
   for (( c=1;c<=i;c++ ))
   do
   echo -n " *"
   sum=`expr $sum + 1`
   done
echo ""
done
d_max=`expr $max - 1`
for (( i=$d_max;i>=$min;i--))
do
   for (( j=i;j<=$d_max;j++ ))
   do
   if [ $j -eq $d_max ]
   then
   echo -n " "
   fi
   echo -n " "
   done
   for (( c=1;c<=i;c++ ))
   do
   echo -n " *"
   sum=`expr $sum + 1`
   done
echo ""
done
echo "Total No. of stars : "  $sum

Output:

      *
     * *
    * * *
   * * * *
  * * * * *
   * * * *
    * * *
     * *
      *

Shell programs should start with a shebang, so that when executed, the OS can select the correct interpreter:

#!/bin/bash

With Bash, we can read prompted input using read -p:

read -p "Enter the mininum number of stars: " min

We should check that we have a valid positive integer before using $min.

The same comments also apply to $max.  For both variables, consider accepting arguments instead of prompting for interactive input.

Prefer printf to echo -n - that's a good practice for portable scripting.

Most of those for loops can be eliminated with suitable printf formatting.  We can produce n stars with printf '%.${n}s' '' | tr ' ' '*', and we can put that into a space that's m wide, with printf '%${m}s' "$(printf '%.$ns' '' | tr ' ' '*')".  If we did keep the loops, we don't need $ for variable expansion within the arithmetic context (( )).

It seems strange to use arithmetic evaluation but also to invoke expr to increment $sum - it's more consistent to simply write ((++sum)).

The echo command with a single empty argument is equivalent to echo with no arguments, so no need to write "" there.


Initializing complex constant in Constants interface I'm being forced to use the common anti-pattern of a public interface Constants which many classes in this project implement.  Long story short, I need to have a List constant which is pre-populated with values.  Normally I would do this like so:

public static final List<String> MY_CONSTANT = new ArrayList<String>();
static {
    MY_CONSTANT.add("foo");
    MY_CONSTANT.add("bar");
    // ...
}

Unfortunately, I can't use static initializers in an interface.  So after much frustration, I finally have my implementation as follows:

public static final List<String> MY_CONSTANT = new ArrayList<String>() {
    private static final long serialVersionUID = 1898990046107150596L;
    {
        add("foo");
        add("bar");
        // ...
    }
}

I hate the fact that I've made an anonymous extension like this and it makes me cringe.  Are there any better techniques I can use to accomplish this?  Changing the Constants file to a class instead of an interface isn't an option.  As I said, I'm just retouching a large, pre-existing code base.

If you only need constant of java.util.List type you can use static method java.util.Arrays.asList:

import static java.util.Arrays.asList;

...

public static final List<String> MY_CONSTANT = asList("foo", "bar", "baz");

which creates immutable list instance with passed values.

In cases when more complex object needs to be initialised I like to use static methods which create my constant:

public static final SomeComplexObject constant = createMyConstant();

private static SomeComplexObject createMyConstant() {
    SomeComplexObject constant = new SomeComplexObject();
    constant.setProp1(...);
    constant.setProp2(...);
    ...
    return makeItSomehowImmutable(constant);
}

In your case you won't be able to declare private static method inside interface, so the only thing I find reasonable is to create separate utility class with static methods which create constants, declare your method there and reuse it just like with java.util.Arrays.asList method.

Hope this helps...


Adding a duplicate entry randomly into a list in haskell using random monad There's a new version of this as v2 - Adding a duplicate entry randomly into a list in haskell using random monad

I wrote this trying to set up a Haskell testcase.  The aim is to take a list and add a single duplicate from any place in the list to anywhere else in the list.

I'm trying to learn to use the Random Monad properly so the main aims should be clear, simple, idiomatic and pure code.  However any recommendations for improvement are appreciated.

-- duplicate-to-list-randmonad.hs by Michael De La Rue 2014
-- licensed to StackEschange under cc by-sa 3.0
-- may also be used under AGPLv3
-- N.B. Trivial copying of code fragments does not normally require any license.

import Data.List
import Control.Monad.Random

main :: IO ()
main = do putStrLn $ "list comparison " ++ prettyList list
          g <- getStdGen
          let shuffled = evalRand (infiniteDuplicateLists list) g
          putStrLn $ "lists after \n" ++ intercalate "\n" ( map prettyList (take 5 shuffled))
          where
            list = ["a","b","c"]

infiniteDuplicateLists genlist = do
   firstelt <- addRandomDuplicate genlist
   restoflist <- infiniteDuplicateLists genlist
   return $ firstelt : restoflist

prettyList :: (Show a) => [a] -> String
prettyList list = " [ " ++ intercalate "," (map show list) ++ " ] "

addRandomDuplicate :: MonadRandom m => [a] -> m [a]
addRandomDuplicate genlist = do
     frompos <- getRandomR (0 ,length genlist - 1)
     topos   <- getRandomR (0 ,length genlist)
     let  newlist = listEntryDuplicate (fromIntegral frompos) (fromIntegral topos) genlist
     return newlist

listEntryDuplicate from to list =
      start ++ [repeat] ++ end
    where
      repeat = list !! from
      (start, end) = splitAt to list

(edited to move fixed code to a new question version for neatness)

It's very good that you structured your code into several smaller functions.

Some ideas for improvement:

Style: It's very useful to stick to a particular style guide and maximum line width. Also I'd recommend to avoid

name = do exp1
          exp2

in favor of

name = do
  exp1
  exp2

because if you need to change name, you need to change the whole code block (and your code is indented unnecessarily to the right).
Do include signatures for all top-level functions. This makes code much more readable and safer (because the compiler helps you typecheck that the function do what you intended). And, it helps to specialize functions properly, for example, with properly typed listEntryDuplicate you won't need those fromIntegral calls.
Function infiniteDuplicateLists can be expressed using mapM and repeat as

infiniteDuplicateLists :: (MonadRandom m) => [a] -> m [[a]]
infiniteDuplicateLists = mapM addRandomDuplicate . repeat

However note that it relies on the fact that MonadRandom allows to traverse infinite lists, which is a bit strong assumption. While it works, I don't think MonadRandom gives any guarantees that it will work in the future. Traversing infinite lists works generally only for some specific monads, see also Why the Haskell sequence function can't be lazy or why recursive monadic functions can't be lazy? Instead I'd recommend sticking to finite lists, like

duplicateLists :: (MonadRandom m) => Int -> [a] -> m [[a]]
duplicateLists n = replicateM n . addRandomDuplicate

In addRandomDuplicate you unnecessarily compute the length of the list twice.

The full code could look like

import Data.List
import Control.Monad
import Control.Monad.Random

main :: IO ()
main = do
    putStrLn $ "list comparison " ++ prettyList list
    g <- getStdGen
    let shuffled = evalRand (duplicateLists count list) g
    putStrLn $ "lists after \n"
               ++ intercalate "\n" (map prettyList shuffled)
  where
    count = 5
    list = ["a","b","c"]

duplicateLists :: (MonadRandom m) => Int -> [a] -> m [[a]]
duplicateLists n = replicateM n . addRandomDuplicate

prettyList :: (Show a) => [a] -> String
prettyList list = " [ " ++ intercalate "," (map show list) ++ " ] "

addRandomDuplicate :: MonadRandom m => [a] -> m [a]
addRandomDuplicate genlist = do
     let l = length genlist
     frompos <- getRandomR (0, l - 1)
     topos   <- getRandomR (0, l)
     return $ listEntryDuplicate frompos topos genlist

listEntryDuplicate :: Int -> Int -> [a] -> [a]
listEntryDuplicate from to list =
      start ++ [repeat] ++ end
    where
      repeat = list !! from
      (start, end) = splitAt to list

Another room for improvement is replacing lists with a better data structure. For small lists it's not necessary, but if you are going to use longer lists, I'd definitely recommend using Seq, whose operations have only O(log n) time complexity as opposed to O(n) for lists. (See also How fast is Data.Sequence.Seq compared to []?)