Generating a list of powers in Elm up to some limit I'm looking to generate a list of powers in Elm, up to a certain limit. I've written the following tests:

        [ test "works for negative limit" <| \_ -> equal (powers 2 -5) (Ok [])
        , test "works for 2 up to 63" <| \_ -> equal (powers 2 63) (Ok [1, 2, 4, 8, 16, 32])
        , test "works for 2 up to 64" <| \_ -> equal (powers 2 63) (Ok [1, 2, 4, 8, 16, 32])
        , test "works for 3 up to 81" <| \_ -> equal (powers 3 81) (Ok [1, 3, 9, 27, 81])
        , test "rejects negative bases" <| \_ -> equal (powers -2 63) (Err "negative base")
        ]

I have a solution that I'm not happy with because it seems pretty long. Now I know that when doing these kinds of things in the functional style, we generally use tail recursion (which is no problem). But as I am building up the list from small numbers to large numbers (e.g., 1 2 4 8 16 32 ...) I could keep appending to the accumulated list, but this is much slower than using prepend (::). So... I decided to prepend and then reverse at the very end:

powers base limit =
  if base < 0 then
    Err "negative base"
  else
    let
      helper base exponent limit soFar =
        let next = base ^ exponent in
          if next > limit then
            Ok soFar
          else
            helper base (exponent + 1) limit (next :: soFar)
    in
      case helper base 0 limit [] of
        Ok powerList -> Ok <| List.reverse powerList
        Err message -> Err message

This passes all the tests but I am wondering two things:

Is consing (::) and then reversing at the end the right way to do this or is there a better way?
Is there something amazing in Elm's List module that I missed that I should be using here?
At the very end, when I wanted to reverse, was it necessary to do that matching? It seems unnecessarily verbose because I had to pass through the whole Err part. Since I only wanted to reverse a list in the Ok case and leave the Err part unchanged, was there a better way to write this?

Why don't you just use the exponentiation (^) operator? Using that you can omit the recursion at all:

powers : Int -> Int -> Result String (List Int)
powers base limit =
    if base < 0 then
        Err "negative base"
    else
        let size = floor (logBase (toFloat base) (toFloat limit))
        in Ok (List.map (\i -> base ^ i) (List.range 0 size))

There are some functions I used here:

logBase calculates the logarithm of a number with a given base. This can help to calculate the size of the returned list.
List.range : Int -> Int -> List Int generates an Int list where every element increasing by one.


Assign multiple values to variables with a function in a class I would like to use this feature

>>> def func():
>>>     return 1, 2
>>> x, y = func()

But I have a formatting problem, because I want to assign values in the __init__() of a Class.

import csv

class Filewriter:
    def __init__(self, jsonstring, filepath):
        self.katAsk, self.katBid, self.promAsk, self.promBid, self.montAsk, self.montBid, self.katProm, self.katMont, self.promKat, self.promMont, self.montKat, self.montProm, self.ticker = self.get_row()

        self.jsonstring = jsonstring
        self.filepath = filepath

    def jsonparse(self, ticker, label):
        try:
            output = self.jsonstring[ticker][label]
        except (KeyError, ValueError) as e:
            output = None
        return output

    def test(self, teststring, ticker, cx):
        return self.jsonparse(ticker, cx+"."+teststring)

    def get_row(self, ticker):

        checkdic = {"ticker": ticker}
        # Check if asks/bids exist
        checkdic["cx"] = "CI1"
        try:
            katAsk = float(self.test("ask", **checkdic))
        except (TypeError, ValueError) as e:
            katAsk = -1

        try:
            katBid = float(self.test("bid", **checkdic))
        except (TypeError, ValueError) as e:
            katBid = -1

        checkdic["cx"]="IC1"
        try:
            promAsk = float(self.test("ask", **checkdic))
        except (TypeError, ValueError) as e:
        promAsk = -1
        try:
            promBid = float(self.test("bid", **checkdic))
        except (TypeError, ValueError) as e:
            promBid = -1

        checkdic["cx"]="NC1"
        try:
            montAsk = float(self.test("ask", **checkdic))
        except (TypeError, ValueError) as e:
            montAsk = -1
        try:
            montBid = float(self.test("bid", **checkdic))
        except (TypeError, ValueError) as e:
            montBid = -1

        # bidDest - askSource = sales
        # Determine sales
        # Kat -> Prom
        if(promBid > 0 and katAsk > 0):
            katProm = promBid - katAsk
        else:
            katProm = -1

        # Kat -> Mont
        if(montBid > 0 and katAsk > 0):
            katMont = montBid - katAsk
        else:
            katMont = -1

        # Prom -> Kat
        if(katBid > 0 and promAsk > 0):
            promKat = katBid - promAsk
        else:
            promKat = -1

        # Prom -> Mont
        if(montBid > 0 and promAsk > 0):
            promMont = montBid - promAsk
        else:
            promMont = -1

        # Mont -> Kat
        if(katBid > 0 and montAsk > 0):
            montKat = katBid - montAsk
        else:
            montKat = -1

        # Mont -> Prom
        if(promBid > 0 and montAsk > 0):
            montProm = promBid - montAsk
        else:
            montProm = -1

        return katAsk, katBid, promAsk, promBid, montAsk, montBid, katProm, katMont, promKat, promMont, montKat, montProm, ticker

    def tablewriter(self):
        with open(self.filepath, mode='w', newline='') as ag:
            agWriter = csv.writer(ag, delimiter=',',
                                  quotechar='"',
                                  quoting=csv.QUOTE_MINIMAL)

            agWriter.writerow([self.ticker,
                               self.katAsk,
                               self.katBid,
                               self.promAsk,
                               self.promBid,
                               self.montAsk,
                               self.montBid,
                               "",
                               self.katProm,
                               self.katMont,
                               self.promKat,
                               self.promMont,
                               self.montKat,
                               self.montProm,
                               "",
                               ])

Is it possible to format this statement on more than one line ?

edit: edited with real code

edit2: noticed a missing "self." before test() calls

Use parens:

(self.katAsk, self.katBid, self.promAsk, self.promBid, self.montAsk,
 self.montBid, self.katProm, self.katMont, self.promKat, self.promMont,
 self.montKat, self.montProm, self.ticker) = self.get_row()


Password-generation function using custom seed After reading an article on Skull Security mentioning the potential weakness of php's mt_rand function because of weak auto-seeding (http://ow.ly/4nrne), I decided to see what -- if any -- entropy I could find available from within php. The idea is to have enough (weak) sources that even if one or two are manipulated, lost or recovered, there's enough left to thwart brute force against the resulting passwords later.

Hopefully the result is both readable and usable, although I don't expect it to be production-quality.

<?php
  /**
   * Return a random password.
   *
       * v1.01
   * Jumps through many hoops to attempt to overcome autoseed weakness of php's mt_rand function
   *
   */
  function myRandomPassword() {
        // Change this for each installation
        $localsecret = 'qTgppE9T2c';
    // Determine length of generated password
    $pwlength = 10;
    // Character set for password
    $pwchars = 'ABCDEFGHJKLMNPQRSTUVWXYZabcdefghjkmnpqrstuvwxyz0123456789';
    $l = strlen( $pwchars ) - 1;
    // Get a little bit of entropy from sources that should be inaccessible to outsiders and non-static
    $dat = getrusage(); // gather some information from the running system
    $datline = md5(implode($dat)); // wash using md5 -- it's fast and there's not enough entropy to warrant longer hash
    $hardToGuess = $datline;
    $self = __FILE__; // a file the script should have read access to (itself)
    $stat = stat($self); // information about file such as inode, accessed time, uid, guid
    $statline = md5(implode($stat)); // wash
    $hardToGuess .= $statline;
    $preseed = md5(microtime()) . getmypid() . $hardToGuess . memory_get_usage() . disk_free_space('.') . $localsecret;
    $seed = sha1( $preseed ); // final wash, longer hash
    // Seed the mt_rand() function with a better seed than the standard one
    mt_srand ($seed);
    // Pick characters from the lineup, using the seeded mt_rand function
    $pw = '';
    for ( $i = 0; $i < $pwlength; $i++ ) {
      $pw .= $pwchars{ mt_rand( 0, $l ) };
    }
    // Return the result
    return $pw;
  }

echo myRandomPassword();
?>

Revision 1.01 adds a local secret.

I actually don't get the point of injecting so much system information into the mt_srand function. Looks like a total (and maybe even pointless) paranoia :)

But here you go with a cleaner code:

<?php
/**
* Random password generator
* v2.0
*/

define('APP_SECRET_KEY', 'qTgppE9T2c');

function randomPassword($length=10) {
  $charset = 'ABCDEFGHJKLMNPQRSTUVWXYZabcdefghjkmnpqrstuvwxyz0123456789';
  $charsetSize = strlen($charset) - 1;

  // Seeding the generator with a bunch of different system data and the secret key
  mt_srand(crc32(md5(microtime())
    . getmypid()
    . md5(implode(getrusage()))
    . md5(implode(stat(__FILE__)))
    . memory_get_usage()
    . disk_free_space('.')
    . APP_SECRET_KEY)
  );

  $password = '';
  foreach (range(1, $length) as $_)
    $password .= $charset{mt_rand(0, $charsetSize)};

  return $password;
}

echo randomPassword(), "\n";

Maybe you'll like the more perverted superslow version which returns CRC32 of randomly ordered entropy each time you generate a new symbol.

<?php
/**
* Random password generator
* v2.1
*/

function randomPassword($length=10) {
  $charset = 'ABCDEFGHJKLMNPQRSTUVWXYZabcdefghjkmnpqrstuvwxyz0123456789';
  $charsetSize = strlen($charset) - 1;

  $seeders = array(
    function () { return md5(microtime()); },
    function () { return md5(getmypid()); },
    function () { return md5(implode(getrusage())); },
    function () { return memory_get_usage(); },
    function () { return disk_free_space('.'); }
  );

  $randomSeed = function () use ($seeders) {
    shuffle($seeders);

    $entropy = '';
    foreach ($seeders as $seeder)
      $entropy .= $seeder();

    return crc32($entropy);
  };

  $password = '';
  foreach (range(1, $length) as $_) {
    mt_srand($randomSeed());
    $password .= $charset{mt_rand(0, $charsetSize)};
  }

  return $password;
}

echo randomPassword(), "\n";


Project Euler - multiples of numbers I'm trying to learn functional programming and Scala, following through Project Euler problems.

I'm solving get the sum of multiples x & y in a list. So for a list [1,2,3,4,5,6,7,8,9] get multiples of 3, 5 returns 3, 5, 6 and 9, the sum being 23.

I'm accepting two parameters list and multiples
Mapping multiples to list of functions, that test if input is a multiple
Filtering anything that returns true in the mapped multiples list

  def multiplesOf(list: List[Int], multiples: List[Int]): List[Int] = {
    def filterMultipleOf(x: Int) : Int => Boolean = (y: Int) => {
      y % x == 0
    }
    val multiplesFilter = multiples.map(filterMultipleOf(_))
    list.filter( int => multiplesFilter.exists(mOf => mOf(int)))
  }

scala> multiplesOf(List.range(1,10), List(3,5)).sum
res: Int = 23

My main question: Is this the Scale/FP way?

Is defining filterMultipleOf inside my function still pure? Is there a better way to express this in Scala or FP?

Yes, your function is still pure. A pure function has only two requirements:

The return value will always be the same given the same arguments.
Evaluation will produce no side effects.

Since filterMultipleOf only uses integers, the second is obviously met, no mutation is possible. The first requirement is also met. Calling filterMultipleOf(1) creates a function which is distinct from the function created by calling filterMultipleOf(2). See this question on software engineering for an alternative explanation.

Is there a better way to express this function? Yes.

Just because your functions are short doesn't mean stop thinking about names. x and y are about as vague as you can get. Renaming x to factor and y to multiple immediately makes the function clearer. Also, multiplesOf takes a list of factors, not a list of multiples.

Naming variables after their type is generally a bad idea. int should be called multiple. list is a bit more difficult. In this case I would leave it as is since I can't quickly come up with a better name (numbers really doesn't help much).
You can pass functions around without specifying a single partial. Do multiples.map(filterMultipleOf) instead of multiples.map(filterMultipleOf(_))
I don't see the need for filterMultipleOf, you can just use an anonymous function without losing any meaning. factors.map(factor => (multiple: Int) => multiple % factor == 0)
Choosing to create a function for each factor makes your function logic take more space since you now have to call each function. I believe it is more clear to create a helper function that checks all factors:

def multiplesOf(list: List[Int], factors: List[Int]): List[Int] = {
  val isMultiple = (multiple: Int) => factors.exists(factor => multiple % factor == 0)
  list.filter(isMultiple)
}

You could further reduce this function by providing isMultiple as a lambda, but this is really dependent on your style. I'd go for it in this case.

def multiplesOf(list: List[Int], factors: List[Int]): List[Int] = {
  list.filter(multiple => factors.exists(factor => multiple % factor == 0))
}

It might be worth questioning the public API for this function. As it stands, it is only useful for lists of integers. What happens if you have a list of another type - say, Person and you want all people whose age is a multiple of 3 or 5? Right now, you can't use this function.

If the function was slightly modified to tell if a single integer is a multiple of some factor, then you can use the function.

val multipleOf = (multiple: Int, factors: List[Int]) =>
  factors.exists(factor => multiple % factor == 0)

class Person(var name: String, var age: Int) {
  override def toString: String = s"$name is $age"
}

val people = List(
  new Person("Adam", 25),
  new Person("Bob", 7),
  new Person("Cate", 12),
  new Person("Dave", 20),
  new Person("Edward", 8)
)

people
  .filter(person => multipleOf(person.age, List(3, 5)))
  .foreach(println)

With this change, your original function can be trivially defined:

val multipleOf = (multiple: Int, factors: List[Int]) =>
  factors.exists(factor => multiple % factor == 0)

val multiplesOf = (list: List[Int], factors: List[Int]) =>
  list.filter(multipleOf(_, factors))


boost::unordered_set intersection using templates I wrote this function to do unordered_set intersection using templates. I have seen this answer but I thought that it was overkill. I would like the method to take in 2 sets and return a set.

class SetUtilites{
public:
    template<typename Type>
    static boost::unordered_set<Type> intersection(const boost::unordered_set<Type> &set1, const boost::unordered_set<Type> &set2){
        if(set1.size() < set2.size()){

            boost::unordered_set<Type> iSet;

            boost::unordered_set<Type>::iterator it;
            for(it = set1.begin(); it != set1.end();++it){
                if(set2.find(*it) != set2.end()){
                    iSet.insert(*it);
                }
            }
            return iSet;
        }else{
            return intersection(set2,set1);
        }
    }
};

SetUtilites is misspelled; it should be SetUtilities.
For better readability, consider doing something about this long line:

static boost::unordered_set<Type> intersection(const boost::unordered_set<Type> &set1, const boost::unordered_set<Type> &set2)

You could shorten the types as such via typedef, but it may not help very much, or it may even make things uglier.  You could also wrap the line in some way.  Choose whichever works best.
Do you even need a class for this?  It just contains one function and no data members, so it's not really doing anything relevant as one.  Instead, just make this a free function (non-member function) and put it into a namespace with a similar name as the class.
The name iSet sort of sounds like Hungarian notation, although it appears to be a shortened form of intersectionSet.  If you still don't want to call it something that lengthy, you could rename it to something like finalSet, indicating that it will be returned from the function.


Modification of javascript inheritance framework I've modified John Resig simple javascript inheritance framework, by adding set function with setter functionality. It seems that it works. Is it written correctly? Can you see some undesired behavior?
Framework:

(function(){
   var initializing = false, fnTest = /xyz/.test(function(){xyz;}) ? /\b_super\b/ : /.*/;
   this.Class = function(){};
   Class.extend = function(prop) {
     var _super = this.prototype;
    initializing = true;
    var prototype = new this();
    initializing = false;
    for (var name in prop) {
      prototype[name] = typeof prop[name] == "function" &&
        typeof _super[name] == "function" && fnTest.test(prop[name]) ?
        (function(name, fn){
          return function() {
            var tmp = this._super;
            this._super = _super[name];
            var ret = fn.apply(this, arguments);
            // CHANGED HERE FROM this._super = tmp;
            // don't like some not used properties
            if(tmp)this._super = tmp;
            // END OF CHANGE
            return ret;
          };
        })(name, prop[name]) :
        prop[name];
    }
    function Class() {
      if ( !initializing && this.init )
        this.init.apply(this, arguments);
    }
    Class.prototype = prototype;
    Class.prototype.constructor = Class;
    Class.extend = arguments.callee;

    // CHANGED HERE
    // Set method
    Class.prototype.set = function(attrs) {
      for (var attr in attrs) {
        //if exist setter function this.on_change{key of wrap: function(key of wrap, new value)
        if(this.on_change[attr])this.on_change[attr].call(this,attr,attrs[attr]);
        else this[attr] = attrs[attr];
      }
      return this;
    };
    if(!Class.prototype.on_change)Class.prototype.on_change={};
    // END OF CHANGE

    return Class;
  };
})();

Test code:

var Person = Class.extend({
  a: 15,
  b: 30,
  init: function(){
      //...
  },
  on_change:{
      b: function(oldKey, set){this[oldKey] = set*2}
  }
});
var p = new Person();
p.set({a:25,b:40});

Easy question first : Is it written correctly?

Class.prototype.set = function(attrs) {
  for (var attr in attrs) {
    //if exist setter function this.on_change{key of wrap: function(key of wrap, new value)
    if(this.on_change[attr])this.on_change[attr].call(this,attr,attrs[attr]);
    else this[attr] = attrs[attr];
  }
  return this;
};
if(!Class.prototype.on_change)Class.prototype.on_change={};

attrs is not a good name, it does not contain a list of attributes, but a list of name/value pairs, I would suggest the Spartan o for Object or maybe map.
Dropping the curly braces in your if statement is ok, dropping the newlines not so much
The one line of comment is not very helpful :\
You could use the following for initialization of on_change :
Class.prototype.on_change = Class.prototype.on_change || {};
on_change is not lowerCamelCase -> onChange would be more idiomatic.

I would counter suggest:

Class.prototype.set = function( map ){
  for ( var attributeName in map ) {
    var value  = map[attributeName],
        setter = this.onChange[attributeName];
    if( setter )
      setter.call( this, attributeName, value );
    else
      this[attributeName] = value;
  }
  return this;
};
Class.prototype.onChange = Class.prototype.onChange || {};

Hard question: Can you see some undesired behavior?

Yes.

Providing onChange is old skool, real events use addEventListener.
There are now 2 ways to change a property, thru set and thru changing properties directly. The setter functions only work for set, this will introduce bugs.
You are counting on the setter to do the assignment,  another possible source of bugs. I would do the assignment regardless, rename setter to listener and then execute the listener:

this[attributeName] = value;
if( listener )
  listener.call( this, attributeName, value );


Assembly beginner exercise: Calculate the square numbers from 1 to n I try to learn (flat-, FASM-) Assembly currently.

For to become more familiar with the concept of branching I made a tiny program. Inspired by an exercise I had to do once in school when learning PHP.

"Write a program which takes an integer given by the user. The program then calculates and prints the square numbers from 1 to the given number."

Here my source code.
I have added comments in the code. Trying to explain my thoughts and motivations.

format PE console
entry start

include 'win32a.inc'

; ===========================================
; = Displays the square number from 1 until =
; =  number n. n is given by the user.      =
; = @param [ integer ] n - The upper        =
; =  limit. The last integer to calculate   =
; =  the square number of.                  =
; ===========================================

; - Example --------------------------------
; 100 // User have entered 100.
; 1
; 4
; 9
; 10  // 16 in base 10
; 19  // 25 in base 10
; 24  // 36 in base 10
; 31  //   ...
; 40
; 51
; 64
; 79

section '.text' code readable executable

start:
    call    read_hex ; Provided by teacher. Reads in a hexadecimal number from stdin.

    mov     ecx, eax
    mov     ebx, 1 ; First number ...

    cmp eax, 1000000 ; When the number received is smaller the upper limit ...
    jle createSquareNumbers ; ... then you do not have to override it.

    mov     ecx, ebp ; ... otherwise you override it with the upper limit.

createSquareNumbers:
    mov edx, 0
    mov eax, ebx
    mul ebx ; Multiply the number with itself.

    call print_eax ; ; Provided by teacher. Prints eax to stdin.

    inc ebx ; Go to the next integer.

    cmp ebx, ecx
    jle createSquareNumbers

    push    0
    call    [ExitProcess]

include 'training.inc'

It seems to work as expected.

But I'm sure there's a lot of room for improvements ...

So therefore: Hints and comments from more experienced Assembly programmers very appreciated. :)

The biggest problem here—and generally with assembly—is the use of registers in the code without explicit documentation of what those registers contain, e.g., in the form of comments. Certain assumptions can be made based on common calling conventions (like that functions will return their result in eax), but that should still be documented for clarity. In other cases, it is not quite so obvious. For example, in this code, ebp apparently contains the "upper limit", but other than assuming that the code works and reverse-engineering its logic, I have no basis for that assumption. In fact, in the current implementation, the contents of the ebp register are a pre-condition for the function, so its use should unarguably be documented as part of the function's signature.

I further assume that ebp is actually just 1000000, in which case this portion of your code is overly verbose (and thus inefficient). You can simply do:

cmp  eax, 1000000          ; see if input is over the upper limit
jle  createSquareNumbers   ; if not, skip the next line
mov  eax, 1000000          ; if it is, bound it

If you are targeting the Pentium Pro or later (so basically, any modern x86 processor) you can use conditional move instructions like so:

cmp    eax, ebp
cmovg  eax, ebp

In some cases, conditional moves will make the code faster by eliminating the possibility of branch mispredictions. In this case, you won't see any performance improvement, but these instructions are still something to be aware of and this does make the code slightly easier to read. (Assuming that you want to keep the dependency on the ebp register, which might be handy if you want to make the upper limit variable. Otherwise, just hard-code the 1000000 and use the first version. Better yet, define a constant for that upper bound so you can give it a readable name.)

mov edx, 0

This is completely correct code for setting a register to 0. However, it is not idiomatic x86 assembly because it is inefficient. Both compilers and expert assembly-language programmers will instead write this as:

xor  edx, edx

This works because bitwise XORing any value with itself gives you back 0. The advantage is that it is shorter (2 bytes instead of 5 bytes) and faster.

The only time you would ever want to use mov  reg, 0 is if you didn't want to clobber the flags, which is handy when you're preparing to do a conditional move and in certain other tightly hand-optimized code. Otherwise, whenever you want to clear a register, think xor  reg, reg. While this is arguably a micro-optimization, you really do need to know this idiom so you can read other peoples' (and compilers') assembly code, so you might as well use it yourself.

I can tell that you're working with signed integer values because you're using the l and g condition codes. If you were working with unsigned integer values, you'd be using the b and a condition codes. Therefore, you should be doing a signed integer multiply (imul instead of mul). In fact, imul is preferable for a number of reasons, chief among them is the fact that it has a normal, two-operand form that works just like any other instruction, which means you don't have to worry about the hard-coded register operands that mul requires. This makes the code easier to optimize and, frankly, easier to read. You can use imul for unsigned multiplications, too, as long as you aren't worried about overflow, since the lower 32 bits are always the same. (And clearly you aren't worried about that here, since your print_eax function used to display the output can only print 32-bit values!)

There is, unfortunately, one bug lying in wait here:

You are only checking the upper bound, so what happens when the input value is 0? Your code will print 1, when instead it should print nothing! Also, I wonder if it is possible for read_hex to return a negative value? I don't have the function's implementation, or any documentation for it, so I can't tell. If you're not confident that it won't ever return a negative value, your code needs to handle this case. Fortunately, you can fix both of these potential problems easily by adding a check for the lower bound before entering the loop.

And while we're nitpicking, some of your comments have "bugs", too:

call print_eax ; ; Provided by teacher. Prints eax to stdin.

That should be stdout, not stdin! :-)

; ===========================================
; = Displays the square number from 1 until =
; =  number n. n is given by the user.      =
; = @param [ integer ] n - The upper        =
; =  limit. The last integer to calculate   =
; =  the square number of.                  =
; ===========================================

Your function does not actually take a parameter. Rather, it retrieves the input by calling the read_hex function. Therefore, this documentation is wrong, because it suggests that the function should be able to read a parameter from the normal register used for passing parameters in whatever calling convention it uses, which is not correct. It is important to document your functions, and it's important to document them accurately. In actuality, the function neither receives nor returns any values.

In terms of readability, you should align your code into vertical columns so that instruction opcodes are clearly separated from their operands. This makes it much easier to read and scan. Also, you should either vertically align your end-of-line comments, or put them above the code.

I would also rename your createSquareNumbers label to printSquares, since (1) it prints output rather than simply creating things, and (2) "numbers" is redundant. I also personally like to use PascalCase for labels to make them distinct, but whichever is fine, as long as you're consistent.

Finally, I would prefer to use the ecx register as my loop "counter", since that's what it was originally intended for (the "c" stands for "counter"). Technically, this doesn't matter at all in modern x86 code, and you can treat all of the registers as general-purpose registers, using them interchangeably. And, indeed, in optimized code, you will want to do this. But when you're writing it by hand and don't necessarily care about speed, it is just nice and readable to use registers in a predictable way.

Here is how I would personally write the code for your function, taking all of these things that I've mentioned into account:

start:
    ; Provided by teacher: reads in a hexadecimal number from stdin,
    ; returning it in EAX.
    call   read_hex

    ; Check for underflow: if the input value is less than or equal to 0,
    ; we skip the loop altogether.
    ; NOTE: The naive way to do this check is "cmp eax, 0", but as with
    ;       the XOR trick discussed above, "test eax, eax" is both
    ;       shorter and faster, so that's what everyone uses when they
    ;       want to test the value of a register against 0.
    ;       (You still use CMP with memory operands, of course.)
    test   eax, eax
    jle    finished

    ; Check for overflow: if the input value is larger than 1000000,
    ; we bound it to be exactly 1000000.
    cmp    eax, 1000000
    mov    edx, 1000000
    cmovg  eax, edx

    ; Prepare to enter the loop:
    mov    edx, eax        ; make a copy of the input
    mov    ecx, 1          ; initialize our counter

printSquares:
    mov    eax, ecx        ; put the counter into our scratch register, EAX
    imul   eax, eax        ; EAX = EAX * EAX

    ; Print the square (in EAX) by calling teacher-provided function,
    ; which prints the contents of EAX to stdout.
    ; It may or may not clobber EAX (we don't care),
    ; but it does not return a significant value.
    ; TODO: Make sure it doesn't clobber ECX or EDX!
    call   print_eax

    inc    ecx             ; increment our counter

    ; See if we've reached the input value yet.
    ; If not, continue looping.
    cmp    ecx, edx
    jle    printSquares

finished:
    ; We're now done with the loop, so exit.
    push   0
    call   [ExitProcess]

Warning: There is still potentially a bug in here, depending on how the print_eax function behaves. Normally, in most calling conventions, a function is allowed to clobber the eax, ecx, and edx registers. Therefore, to be on the safe side, you would have to assume that print_eax could clobber the values in these registers, meaning that you would have to either pick different registers to store your temporary values in, or you would have to explicitly save the contents of these registers before calling print_eax and restore them afterwards. However, if you know from the documentation or implementation of print_eax that it does not clobber either ecx or edx, then you are safe. Your original code had this bug, too, yet it worked properly, so I assume this is safe.


Optimize Implementation - Wait for all threads using LINQ In my program I read several information's about the system. I read each categories like CPU, memory, services, software and so on with a own thread. The main program have to wait for the longest thread.

To implement this, I use LINQ and the Join function from Thread.

Here is a small part from my code:

var culture = CultureInfo.CreateSpecificCulture("en-US");
var thread_list = new List<Thread>();
var c = new Computer();

var thrma = new Thread(() =>
{
    c.Mainboard = new Mainboard().GetInformation();
});
thrma.CurrentUICulture = culture;
thrma.Start();
thread_list.Add(thrma);

var thrce = new Thread(() =>
{
    c.ProcessorList = new CentralProcessingUnit().GetInformation();
});
thrce.CurrentUICulture = culture;
thrce.Start();
thread_list.Add(thrce);

...

var thrsvr = new Thread(() =>
{
    DeviceService.ScanServices();
});
thrsvr.CurrentUICulture = culture;
thrsvr.Start();
thread_list.Add(thrsvr);

foreach (Thread t in thread_list)
{
    t.Join();
}

// Everyone is done!

Each hardware device is derived from the Hardware class.

What do you think about the solution? Is there a "nicer" or better implementation? I would like to use a function, because the code is very similar, but I'm not sure how to do it. Any idea?

Thanks for your advices.

It's better to use new asynchronous API since it provides more features for combining asynchronous tasks.

Ideally Mainboard, CentralProcessingUnit and DeviceService should expose asynchronous API if a certain operation is not trivial, so (as you mentioned in comments you wrote them) I would implement asynchronous API for them first, e.g.:

public class Mainboard
{
    public Task<string> GetInformationAsync()
    {
        //TODO: extract long-term operation into Task. Try not to span extra threads here
    }
}

Then your method would look like:

Task.WaitAll(
    new Mainboard().GetInformationAsync().ContinueWith(task => c.Mainboard = task.Result),
    new CentralProcessingUnit().GetInformationAsync().ContinueWith(task => c.ProcessorList = task.Result),
    DeviceService.ScanServicesAsync());

If you decide not to implement async API the code can look like:

var c = new Computer();

Task.WaitAll(
    Task.Factory.StartNew(() => c.Mainboard = new Mainboard().GetInformation()),
    Task.Factory.StartNew(() => c.ProcessorList = new CentralProcessingUnit().GetInformation()),
    Task.Factory.StartNew(() => DeviceService.ScanServices()));


Codewars: Prime Streaming (PG-13) I'm timing out on Codewars' Prime Streaming kata. Below is my code:

"use strict";

let _base = [2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47, 53, 59, 61, 67, 71];
let _bufferSize = 1000

class Primes {
   static get base() {
      return _base;
   }

   static get bufferSize() {
      return _bufferSize;
   }

   static set bufferSize (value) {
      _bufferSize = value;
   }

   static sieve(array) {
      for (let p of Primes.base) {
         array = array.filter(n => n % p !== 0);
      }
      return array;
   }

   static sieve_2(array) {

   }

   static * stream() {
      const candidates = [];
      for (let i = 73; i < 5001; i += 2)
         candidates.push(i);
      Primes.base.push(...Primes.sieve(candidates));
      const buffer = Primes.base.slice();

      while (true) {
         let current;
         while (buffer.length > 0) {
            current = buffer.shift();
            yield current;
         }
         let [lower, upper] = [current + 2, current + Primes.bufferSize];
         let basket = [];
         for (let i = lower; i <= upper; i += 2)
            basket.push(i);
         buffer.push(...Primes.sieve(basket));
      }
   }
}

As far as I can tell, changing the buffer size affected nothing. I first generate primes less than 5001 because their upper bound on primes they test is somewhere below 16 million, and I wasn't enthused about doing the investigation work to find a precise bound, so I just went with a loose one.

Your current code, if I understand it, starts by sieving each target without recognizing that if the target is less than the highest cache (or _base) then it is prime if and only if it in the cache.

One option is to keep your primes in a single cache (_base), make sure that the cache is high as your target, and check if your target is in your cache.

The rough code would have only two functions:  one ensures that base array is high enough, and the other returns the value.   Something like:

const primes = [2, 3, 5];

function ensure_primes(until_n) {
    let n = primes.slice(-1) + 1;
    while (n <= until_n) {
        if (primes.every(prime => n % prime != 0)) {
            primes.push(n)
        }
        n += 1;
    }
}

function is_prime(target) {
    ensure_primes(target);
    return target in primes
}

Is this what you were asking?


Validating users with Roles I am using the following to validate my user which works but i wish to incoperate roles so I used the reg iis to add the standard tables aspnet_UsersInRoles.

What I basically want is the ability to have variables canView canDelete canEdit and that I can just access them when my dal is called.

protected void btnLogin_Click1(object sender, EventArgs e)
{
    Users _loginUser = _dal.VerifyPassword(txtUserName.Text, txtPassword.Text);

    if (failedLogin == false)
        lblerror.Text = "Invalid Login";
    else
    {

        UserData userData = new UserData
        {
            FirstName = _loginUser
        };
        FormsAuthenticationTicket ticket = new FormsAuthenticationTicket(
          1,                                     // ticket version
          _loginUser.UserName,                              // authenticated username
          DateTime.Now,                          // issueDate
          DateTime.Now.AddMinutes(30),           // expiryDate
          isPersistent,                          // true to persist across browser sessions
          userData,                              // can be used to store additional user data
          FormsAuthentication.FormsCookiePath);  // the path for the cookie

        // Encrypt the ticket using the machine key
        string encryptedTicket = FormsAuthentication.Encrypt(ticket);

        // Add the cookie to the request to save it
        HttpCookie cookie = new HttpCookie(FormsAuthentication.FormsCookieName, encryptedTicket);
        cookie.HttpOnly = true;
        Response.Cookies.Add(cookie);

        lblerror.Text = "Success!";

        Response.Redirect("default.aspx");
    }

}

My Question is how would i adapt the following routine to inlcude the roles aspnet_UsersInRoles table to check if has administrative privellages Of a guid type of ED85788D-72DA-4D0A-8D5E-B5378FC00592.

public Users VerifyPassword(string userName, string password)
{
    //The ".FirstOrDefault()" method will return either the first matched
    //result or null
    Users myUser = SoccerEntities.Users
        .FirstOrDefault(u => u.UserName == userName
                     && u.password == password);

    if (myUser == null)    //User was not found
    {
        //Proceed with your login process...
        return myUser;
    }
    else    //User was  found
    {
        if (isPlayerdb(userName))
            isPlayer = true;
        if (isAdmindb(userName))
            isAdmin = true;

        return myUser;
        //Do something to let them know that their credentials were not valid
    }
}

Cause at min im doing this setting the isAdmin variable on a other function isAdmindb so thats three hits to db which is not very effiecent

protected bool isAdmindb(string userName)
{

    try{
           Users adminUsers = (from users in SoccerEntities.Users
                       where users.roleId == new Guid("ED85788D-72DA-4D0A-8D5E-B5378FC00592") && users.UserName == userName
                       select users).FirstOrDefault();

        if (adminUsers != null)
            return true;
        else
            return false;

    }
    catch (Exception ex)
    {
        throw new EntityContextException("isAdmin failed.", ex);
    }
}

Edit a had a typo in my return !!!

public Users VerifyPassword(string userName, string password)
{
    //The ".FirstOrDefault()" method will return either the first matched
    //result or null
    Users myUser = SoccerEntities.Users
        .FirstOrDefault(u => u.UserName == userName
                     && u.password == password);

    if (myUser == null)    //User was not found
    {
        //Proceed with your login process...
        return myUser;
    }
    else    //User was  found
    {
        if (isPlayerdb(userName))
            isPlayer = true;
        if (isAdmindb(userName))
            isAdmin = true;

        return myUser;
        //Do something to let them know that their credentials were not valid
    }
}

The comments in this method don't serve any purpose an don't add any value to the code, so get rid of them. Comments should describe why something is done but shouldn't state the obvious.

A class named Users implies to hold a collections of User but in your case it seems to be singular so you should rename this class to User.

Not using braces {} will lead to error prone code. You should use them always although they might be optional.

The else part is redundant because if myUser == null it won't be reached, you should get rid of the else.

If for instance isAdmin had been true before this call it will stay that way which is bad. Just assign the returned value of isAdmindb to the variable. The same goes for isPlayer.

Applying this points except for Users vs User

public Users VerifyPassword(string userName, string password)
{

    Users user = SoccerEntities.Users
        .FirstOrDefault(u => u.UserName == userName
                     && u.password == password);

    if (user == null) { return null; }

    isPlayer = isPlayerdb(userName);

    isAdmin = isAdmindb(userName);

    return user;

}

looks much cleaner, doesn't it ? But hold on, we can do better.

If we take a look at isAdmindb() we notice at first, that the method name doesn't match the NET naming guidelines regarding the used case of the name. Method names should be named using PascalCase casing.

The next thing is that a user is considered an admin if its roleId matches a specific Guid. So let us take this Guid and use it in the VerifyPassword() method. But wait, this doesn't seem right because the responsibility of that method should be only to verify that the used username and password are correct. So there shouldn't be anything related like in the former method with the isAdmin stuff.

So let us change the isAdmindb() method to take a Users opject as a method argument and compare the user's roleId with the said Guid which we extract to a class member constant.

private const Guid adminRoleId = new Guid("ED85788D-72DA-4D0A-8D5E-B5378FC00592");
protected bool IsAdmin(Users user)
{
    return user.roleId == adminRoleId ;
}

now thats short and readable and doesn't need to hit the db.

Now we should take the VerifyPassword() and change it to GetUser() and remove all this stuff about the isAdmin and isPlayer leaving only this

public Users GetUser(string userName, string password)
{

    return SoccerEntities.Users
        .FirstOrDefault(u => u.UserName == userName
                     && u.password == password);
}

Now let us add a FormsAuthenticationTicket GetAuthenticationTicket(Users)  method which will do some of the work which had formerly been done by the eventhandler of that button.

private FormsAuthenticationTicket GetAuthenticationTicket(Users user)
{
    return new FormsAuthenticationTicket(
          version: 1,
          name: user.UserName,
          issueDate: DateTime.Now,
          expiration: DateTime.Now.AddMinutes(30),
          isPersistent: isPersistent,
          userData: user.ToString(),
          cookiePath: FormsAuthentication.FormsCookiePath);
}

as you see by using named arguments there is no need for any comment anymore.

Now the former eventhandler code will look like

protected void btnLogin_Click1(object sender, EventArgs e)
{
    Users user = GetUser(txtUserName.Text, txtPassword.Text);

    if (user == null)
    {
        lblerror.Text = "Invalid Login";
        return;
    }

    FormsAuthenticationTicket ticket = GetAuthenticationTicket(user);

    isAdmin = IsAdmin(user);
    isPlayer = IsPlayer(user); // should be implemented by you in a similiar way

    string encryptedTicket = FormsAuthentication.Encrypt(ticket);

    HttpCookie cookie = new HttpCookie(FormsAuthentication.FormsCookieName, encryptedTicket);
    cookie.HttpOnly = true;
    Response.Cookies.Add(cookie);

    lblerror.Text = "Success!";

    Response.Redirect("default.aspx");

}


React custom hooks using refs as index counters My custom hook useTyper.js ended up as a bit of a mess and hard to read.
It's mimicking a nested for-loop but using React.useRefs to hold on to the indexes between renders. The indexes are the current positions of the outer and inner loop, those often named i and j in traditional nested for loops.
Here's a fancy code sandbox for the mess below.
One thing I'm particularly unhappy with is the naming of stringIdx and stringsIdx. Any suggestion for a change?
useTyper.js
import React from "react";
import useInterval from "./useInterval.js";

/**
 * A simple typewriter that prints strings, one character after another
 * @param {Array<string>} strings - Array of strings to type
 * @param {function<string>} set - A useState setter to update the current string value
 * @param {number} defaultDelay - Delay between updates
 */
const useTyper = (strings, set, defaultDelay = 100) => {
  const [delay, setDelay] = React.useState(defaultDelay);
  // index counters, normally named i and j if this were an imperative loop
  const stringIdx = React.useRef(0);
  const stringsIdx = React.useRef(0);

  const textTyper = () => {
    const currentString = strings[stringsIdx.current];

    set(currentString.substr(0, stringIdx.current + 1));

    if (stringIdx.current > currentString.length) {
      // Done with string
      stringsIdx.current += 1;
      stringIdx.current = 0;
      if (stringsIdx.current > strings.length - 1) {
        setDelay(null); // Ended.
      }
    }
    stringIdx.current += 1;
  };

  useInterval(textTyper, delay);
};

export default useTyper;

For completion, this is the useInterval hook being used by useTyper.
It can be referenced from one of Dan Abramovs blog posts.
useInterval.js
import React from 'react';

const useInterval = (callback, delay) => {
  const savedCallback = React.useRef();

  // Remember the latest callback.
  React.useEffect(() => {
    savedCallback.current = callback;
  }, [callback]);

  // Set up the interval.
  React.useEffect(() => {
    function tick() {
      savedCallback.current();
    }
    if (delay !== null) {
      let id = setInterval(tick, delay);
      return () => {
        clearInterval(id);
      };
    }
  }, [delay]);
};

export default useInterval;

Here is how the useTyper hook can be used.
import React, { useState } from "react";
import "./styles.css";
import useTyper from "./hooks/useTyper.js";
export default function App() {
  const [placeHolder, setPlaceHolder] = useState("");

  const placeHolders = [
    "First input suggestion",
    "Second input suggeston",
    "Creativity flows...   ",
    "Alright, lets leave it here."
  ];

  useTyper(placeHolders, setPlaceHolder);

  return (

        <input
          id="formElement"
          autoComplete="off"
          autoFocus
          size="20"
          type="text"
          placeholder={placeHolder}
        />

  );
}

Again, here's the link to the code sandbox.

Looks to be fairly clean code.
Suggestions

Use string::slice or string::substring versus string::substr. substr isn't deprecated but also isn't recommended for use.

Warning: Although String.prototype.substr(…) is not strictly
deprecated (as in "removed from the Web standards"), it is considered
a legacy function and should be avoided when possible. It is not part
of the core JavaScript language and may be removed in the future. If
at all possible, use the substring() method instead.

setCurrentString(currentString.slice(0, currentStringIndex.current + 1));

Provide clearer parameter names, i.e. stringArray versus just strings, and setString or setCurrentString versus set. set alone can be confusing as it isn't very descriptive, i.e. "is it a mathematical set?" By naming a function with verbNoun it is clearer to a reader what it is and what it does.

const useTyper = (stringsArray, setCurrentString, defaultDelay = 100) => {

stringIdx and stringsIdx can cause mental gymnastics looking for the 's' to differentiate the two. Isolated from one another the names aren't terrible, but when used together within the same function it is more demanding. The names should also align closer to the variable they indicate an index for. Based on the previous parameter naming suggestion I recommend stringArrayIndex and currentStringIndex.
You also appear to do an extra iteration on the currentString with the conditional test currentStringIndex.current > currentString.length. You can increment to the next string when the current string's index is equal to the length, currentStringIndex.current >= currentString.length.
Similar conditional test for checking for the end of the strings array, when the index is. greater then or equal to the length you can quit. stringArrayIndex.current >= stringArray.length

const useTyper = (stringArray, setCurrentString, defaultDelay = 100) => {
  const [delay, setDelay] = React.useState(defaultDelay);
  // index counters, normally named i and j if this were an imperative loop
  const currentStringIndex = React.useRef(0);
  const stringArrayIndex = React.useRef(0);

  const textTyper = () => {
    const currentString = stringArray[stringArrayIndex.current];

    setCurrentString(currentString.slice(0, currentStringIndex.current + 1));

    if (currentStringIndex.current >= currentString.length) {
      // Done with string
      stringArrayIndex.current += 1;
      currentStringIndex.current = 0;
      if (stringArrayIndex.current >= stringArray.length) {
        setDelay(null); // Ended.
      }
    }
    currentStringIndex.current += 1;
  };

  useInterval(textTyper, delay);
};

Good job, nice hook.


Given two words, transform the first one into the second As the title says, the problem I had to do sounded something like this:
We are given two words, S1 and S2. We must transform S1 into S2, using the following operations:

insert: insert a character in S1 on whichever index you wish.
delete: delete a character from S1 from whichever index you wish.
replace: replace a character from S1, with whatever character you wish, on whatever index you wish.

All the operations have a cost of 1. At the end of the program, the total cost of all the operations must be minimum, therefore we must use as few operations as possible.
Example:
S1="carte"
S2="antet"
first operation: delete 'c' at index 0 ; -> S1 = arte
second operation: replace 'r' with n; -> S1 = ante
third operation: add 't' at the end of S1; -> S1=antet

Therefore, we have a total cost of 3.
Here is my code for it:
    package project3;

import java.util.*;

public class Main {
    static String S1 = "antetlllll";
    static String S2 = "antet";

    public static void main(String[] args) {
        StringBuilder sb;
        int c = 0;

        for (int i = 0; i < S2.length(); i++)
            if (i<S1.length())// we make sure that S1 is at least as big as S2
                if(S1.charAt(i)!=S2.charAt(i)){
                if ((i < S1.length() - 1) && (S1.charAt(i + 1) == S2.charAt(i) ) && (S1.charAt(i + 1) != S2.charAt(i+1) ) ) {  // if the char on the next position of S1 is the char we need, we delete the original one
                    sb = new StringBuilder(S1);
                    sb.deleteCharAt(i);
                    S1 = sb.toString();
                    c++;
                }
                else {  // else, we simply replace it
                    sb = new StringBuilder(S1);
                    String S = "";
                    S = S + S2.charAt(i);
                    sb.replace(i, i+1, S);
                    S1 = sb.toString();
                    c++;
                } }
            else
                if(S1.charAt(i) != S2.charAt(i))
                {  // if S1 is smaller than S2, we simply insert the needed characters into S1
                    sb = new StringBuilder(S1);
                    sb.insert(i, S2.charAt(i));
                    S1 = sb.toString();
                    c++;
                }

        if (S1.length() > S2.length()) {  // at this point S1 has a substring S2 in it. We just remove the extra letters from it.

            int n = S1.length();
            for (int i = S2.length(); i < n; i++) {
                sb = new StringBuilder(S1);
                sb.deleteCharAt(S2.length());
                S1 = sb.toString();
                c++;
            }

        }

        if (S1.length() < S2.length()) {  // at this point S1 has most of S2 characters. We just add the ones still needed.

            int n = S1.length();
            for (int i = n; i < S2.length(); i++) {
                sb = new StringBuilder(S1);
                sb.insert(i,S2.charAt(i));
                S1 = sb.toString();
                c++;
            }

        }

        System.out.println(c);
        System.out.println(S1);
    }

}

Note that when I say things like "S1 bigger than S2", I refer to their lengths.
So I had this problem as an assignment for school, and one of the requirements was that it had to be done using dynamic programming and it had have a complexity of O(m × n) (m=length of S1 ; n=length of S2) at most. Now, I'm new to DP, started doing it today after a bit of research, so I'd like to know if my code meets the two requirements I had.

The algorithm is wrong and can't be fixed.
It's not dynamic programming, it's greedy: it iterates over the strings once and decides immediately which operation to perform. Consider some test cases:
                 case 1            case 2            partial read

string 1:        abcde             abcde             abc..
string 2:        abxde             abxcd             abx..

optimal          replace c         insert x          ?
strategy         by x              delete e

Both cases are identical in the first three characters, yet the operations to perform are different. They depend on characters not yet read.
Any algorithm that does not take into consideration the full picture before making a single descision must fail at least one of those test cases. Yours fails the second.
Brute force
The naive way to overcome this problem is a brute force algorithm. Evaluate all possible sequences of operations (insertions, deletions, replacements). When all input was read, output the best sequence.
Dynamic programming
Dynamic programming starts out similar to brute force and evaluates all possible operations, but whenever it finds that two sequences of operations yield the same intermediate result, it keeps (and evaluates further) only one of them.


Encoding/decoding protocol messages to/from various bits of hardware The basic idea is for users of these classes to create derived polymorphic protocol specific messages. There is no protocol type in the interface, it is all hidden behind the scenes.  But the serialised messages can be retrieved with the getMessage functions.

Does this look a logical approach?  Any suggestions?

//base protocol class - interface only
//ref_type is a smart pointers class - not really so relevant to my question
class BaseMessage : public ref_type
{
public:
    virtual ~BaseMessage() {}
    virtual const char* ProtocolName() const = 0;
    virtual bool Encode() = 0;
    virtual bool Decode() = 0;
    //for binary protocols
    virtual void getMessage(unsigned char*& serialised, size_t& length) {};
    //for text/xml protocols
    virtual void getMessage(std::string& serialised) {}
};

A specific protocol might then be implemented like this:

class ABCMessage : public BaseMessage {
public:
   ABCMessage(ABC* msg) : m_msg(msg), m_data(0), m_length(0) {}
   ABCMessage(unsigned char* data, int length) : m_length(length), m_msg(0)
   {
      m_data = new unsigned char[length]();
      memcpy(m_data, data, length);
   }
   virtual ~ABCMessage();
   virtual const char* ProtocolName() const;
   virtual bool Encode();
   virtual bool Decode();

   virtual void getMessage(unsigned char*& serialised, size_t& length);

protected:
   ABC* m_msg;
   unsigned char* m_data;
   size_t m_length;
};

OK. I doubt it is irrelevant.

//ref_type is a smart pointers class - not really so relevant to my question

Also smart pointers are very hard to get perfectly correct. The standard ones took years and thousands of people looking at them to get correct. So It is usually preferable to use one of the standard smart pointers rather than a home grown one (especially one that is not being reviewed).

So who owns this pointer?

    virtual const char* ProtocolName() const = 0;

There are no ownership semantics associated with pointers. So After I call this I am not sure who is responsible for deleting it. Use a std::string internally and return a const reference to the string.

Not sure why you would want two methods that look like they do the same thing.

    //for binary protocols
    virtual void getMessage(unsigned char*& serialised, size_t& length) {};
    //for text/xml protocols
    virtual void getMessage(std::string& serialised) {}

Both are going to be inefficient as you are copying across multiple buffers (input stream buffer into this buffer which is then returned and read again to build an object). What you really want is a getMessage() method that returns a fully constructed object from the stream.

This calss:

class ABCMessage : public BaseMessage {

Is totally broken as it contains an owned RAW pointer but does not implement the rule of three.


Count elements in a collection based on a condition [Java 8] Came across this question here: Write a generic method to count the number of elements in a collection that have a specific property (for example, odd integers, prime numbers, palindromes).

Following is my solution:

Counter.java

public class Counter {
    public static <T> long countIf(Collection<T> collection, Predicate<T> predicate) {
        return collection.stream()
                         .filter(predicate)
                         .count();
    }
}

Behaviours.java

public class Behaviours {

    public static boolean checkEvenNumber(final int num) {
        return num % 2 == 0;
    }

    public static boolean checkOddNumber(final int num) {
        return num % 2 != 0;
    }

    public static boolean checkPrimeNumber(final int num) {
        if (num == 0 || num == 1) {
            return false;
        }

        for (int i = 2; i * i <= num; i++) {
            if (num % i == 0) {
                return false;
            }
        }
        return true;
    }

    public static boolean checkPalindrome(final String word) {
        for (int i = 0; i < word.length() / 2; i++) {
            if (word.charAt(i) != word.charAt(word.length() - 1 - i)) {
                return false;
            }
        }
        return true;
    }
}

CounterTest.java

public class CounterTest {

    /**
     * Counting odd numbers.
     */
    @Test
    public void testCountingOddNumbers() {
        System.out.println("testCountingOddNumbers");
        List<Integer> ci = Arrays.asList(1, 2, 3, 4, 5, 6);
        long expected = 3l;
        long actual = Counter.countIf(ci, Behaviours::checkOddNumber);
        Assert.assertEquals(expected, actual);
    }

    /**
     * Counting prime numbers.
     */
    @Test
    public void testCountingPrimeNumbers() {
        System.out.println("testCountingPrimeNumbers");
        List<Integer> ci = Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9);
        long expected = 4l;
        long actual = Counter.countIf(ci, Behaviours::checkPrimeNumber);
        Assert.assertEquals(expected, actual);
    }

    /**
     * Counting palindromes
     */
    @Test
    public void testCountingPalindromes() {
        System.out.println("testCountingPalindromes");
        List<String> cs = Arrays.asList("madam", "test", "tacocat", "hello");
        long expected = 2l;
        long actual = Counter.countIf(cs, Behaviours::checkPalindrome);
        Assert.assertEquals(expected, actual);
    }

}

Output

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.mycompany.demo.assignment001.question001.CounterTest
testCountingPalindromes
testCountingPrimeNumbers
testCountingOddNumbers
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.088 sec

Does my implementation take care of all the scenarios or am I missing something?
Can any other Java 8 concept be used here?
Please also review overall correctness.

The code looks very nice! However, here my small complaints:
Tests
The tests are good, but you're testing the Behaviours implicitly, when you test the Counter. The Counter itself does only the following things: stream, filter with the given predicate, and count. The static methods im Behaviours should be tested separately, as should the logic of the Counter. The problem is, if the test fails, you don't know if the problem is the Counter or the Behaviours. The more "implicit" things you test, the harder is it to find the bug.
You're testing with too many values. The logic itself should be the same with two values (one is a palindrome, one is not) as with four values (two palindromes, two not). If the behaviour for two or four values is different, then it's two different test cases.
The lists to count/filter can have a better name.
To make a test case more readable, it's usually split in the three blocks given/when/then, separated by an empty line. Setup the test data / perform the action / assertion.
Also, I recommend, but that's personal preference, to static import static methods, at least the assertion methods, so it looks a bit more clean.
What's missing (at least what I couldn't find):

You're missing the test for even numbers.
You're missing the test for 0/1 number in testCountingPrimeNumbers.
What happens for an empty String (or string with length 1 or 2) in the palindrome count?

Other

check-prefix: The convention is to use a is prefix, if a method returns a boolean, it's more clear than "check".
test-prefix in test methods: That was the convention for JUnit3 (I think), when annotations weren't a thing. It's not needed anymore, when you use the @Test annotation, so instead of testCountingPrimeNumbers, you can go with countPrimeNumbers.
The JavaDocs for the test methods are obsolete, the method names are clear.
I'd get rid of the System.outs, too, you get enough information in the junit test report.
Instead of naming stuff actual, I'd give it the proper name, e.g. amountOfPalindromes. And maybe not declare the expected variable.

Hope this helps...


Excel VBA Sub Routine is extremely Slow deleting a Range The following script is very slow and I'm not sure why.
The script works so if you copy and paste its a typo in the question.

Sub deleteData()
Dim wb as workbook
Dim ws as worksheet
Dim myCell as range
dim myRange as Range

with Application
 .Calculation = xlCalculationManual
 .ScreenUpdating = False
end with

Set wb = activeworkbook
set ws = wb.Sheets(1)
set myRange = ws.range("I15845:I64572")
set myCell = ws.range("I15845")

For each myCell in myRange
  myCell.Delete
next myCell

with Application
 .Calculation = xlCalculationAutomatic
 .ScreenUpdating = True
end with

MsgBox "Procedure Complete!"

end sub

I have also tried

Sub deleteData()
Dim wb as workbook
Dim ws as worksheet
Dim myCell as range
dim myRange as Range

with Application
 .Calculation = xlCalculationManual
 .ScreenUpdating = False
end with

Set wb = activeworkbook
set ws = wb.Sheets(1)
set myRange = ws.range("I15845:I64572")

myRange.Delete

with Application
 .Calculation = xlCalculationAutomatic
 .ScreenUpdating = True
end with

MsgBox "Procedure Complete!"

end sub

I know this is a large range but it should not take hours to complete.

The data in the column is an anchored countif.

=countif($A$1:A1,A1)

So each cell would be:

=countif($A$1:A1,A1)
=countif($A$1:A2,A2)
=countif($A$1:A3,A3)
.
.
.
=countif($A$1:A10000,A10000)

COUNTIF() is slow.  A few COUNTIF() statements in a worksheet can be okay, but thousands of them will result in a glacial spreadsheet.

Here is a way to replace your COUNTIF() statements:

Sort Column A.
In Column B, have each cell check if the corresponding cell in column A is equal to the cell above it in column A.  If no, put 1 in column B's cell.  If yes, set column B's cell equal to 1 + the value in the cell above it in column B.
In Column C, have each cell check if the corresponding cell in column A is equal to the cell below it in column A.  If yes, let the cell in column C be an empty string.  If no, set column C's cell equal to column B's cell.
(Optional)  In Column D, have each cell check if the corresponding cell in column C is an empty string.  If no, set column D's cell equal to column C's cell.  If yes, set column D's cell equal to the cell below it in column D.


One-shot number guessing game This program is one of the exercises I solved in my book. Can anyone suggest any improvements to it?

import java.util.Random;
import java.util.Scanner;

public class GuessMyNumber {

  public static void main(String[] args) {
    Scanner in = new Scanner(System.in);
    Random random = new Random();

    System.out.println("I'm thinking of a number between 1 and 100 (including both).\nCan you guess what it is?");
    System.out.print("Type a number : ");
    int guessNum = in.nextInt();
    System.out.printf("Your guess is : %d\n", guessNum);
    int randomNum = random.nextInt(100) + 1;
    System.out.printf("The number I was thinking of is : %d\n", randomNum);
    System.out.print("You were off by : ");

    int diff = guessNum - randomNum;
    if (guessNum >= randomNum) {
      System.out.println(diff);
    } else { System.out.println(randomNum - guessNum); }
  }
}

int diff = guessNum - randomNum;
    if (guessNum >= randomNum) {
      System.out.println(diff);
    } else { System.out.println(randomNum - guessNum); }

You mix styles here.  Consider

    if (guessNum >= randomNum) { System.out.println(guessNum - randomNum); }
    else { System.out.println(randomNum - guessNum); }

That's a distinct style.  It puts the {} on the same line as the contained statement and it does every calculation just when it is needed.  Most importantly it's consistent.

I don't like that style.  It's especially uncommon.  I'd prefer

    int difference = guess - solution;
    if (difference >= 0) {
        System.out.println(difference + " high.");
    } else {
        System.out.println(-difference + " low.");
    }

We only calculate the difference once.  Then we use it three times.

This uses a four-column indent, which is standard in Java.

This puts {} on the same lines as their control structures when possible.  And it puts the enclosed statements on lines of their own.  Also standard in Java.

I changed the names of the variables to concentrate on their role in the program.  Not characteristics of how they are defined, e.g. randomly or numeric.  This leaves us with more flexibility to change in the future.  For example, this version could be easily modified to guess letters.

If you don't like the high/low, you could just say

    System.out.println(Math.abs(guessNum - randomNum));

No intermediate variables or if/else that way.


First angularjs directive I've just written my first angularjs directive, and I was hoping to get some feedback. I already have two doubts myself.

Why is it that I must use tElement.append() to append the canvas element? tElement.html() doesn't work, at all. That just returns [[object HTMLCanvasElement]] in plain text inside of the element.
Should I be accessing canvasElement[0]? It feels wrong, but it might not be.

Here's the code:

angular.module('yupDraw', [])
    .directive('yupCanvas', function factory() {

        // Create elements (templates) in vars here
        var canvasTemplate = '<canvas></canvas>';

        return {

            restrict: 'E',
            compile: function compile(tElement, tAttrs, transclude) {

                var canvasElement = angular.element(canvasTemplate);

                tElement.append(canvasElement);

                return function link(scope, element, attrs) {
                    var canvas = new fabric.Canvas(canvasElement[0], {
                        isDrawingMode: true
                    });
                }
            }
        };
    });

And I'm simply calling the directive like so: <yup-canvas></yup-canvas>

Fire away! :)

Angular includes a tiny subset of jQuery called jqLite which is what you get with angular.element.  The html() method on it specifically only accepts a string, not an array of jqLite objects like jQuery normally does I think (unless it's a bug).

One solution would be to use jquery.  If you include jquery before angular, angular will use your jquery instead of the embedded jqlite.  Swap the script tags in this fiddle to see it toggle between working and not.

Another solution would be to use the outerHTML of your canvas element's html node like in this fiddle:

tElement.html(canvasElement[0].outerHTML);

I think using canvasElement[0] is fine.


Imported data from CSV successfully, but slow Currently I am using the gem activerecord-import to seed more than 55,000 records into my database, and it works fine. However, it takes too much time. I am wondering if there are any other ways to shorten the import time. Any suggestions would be appreciated.

ocean_routes_columns = [:origin_location_id, :destination_location_id, :n1st_lane_id, :n1st_pol_id, :n1st_pod_id, :n1st_skd_dir, :n2nd_lane_id, :n2nd_pol_id, :n2nd_pod_id, :n2nd_skd_dir, :n3rd_lane_id, :n3rd_pol_id, :n3rd_pod_id, :n3rd_skd_dir, :created_by_id, :updated_by_id]
ocean_routes = []
CSV.foreach(Rails.root.join('db/seeds/ocean_routes.csv'), headers: true) do |row|
  origin_location            = Location.find_by(code: row[0])
  destination_location       = Location.find_by(code: row[1])
  n1st_lane                  = ServiceLane.find_by(code: row[2])
  n1st_pol                   = Location.find_by(code: row[3])
  n1st_pod                   = Location.find_by(code: row[4])
  n1st_skd_dir               = OceanRoute.to_bound_code_value(row[5])
  n2nd_lane                  = ServiceLane.find_by(code: row[6])
  n2nd_pol                   = Location.find_by(code: row[7])
  n2nd_pod                   = Location.find_by(code: row[8])
  n2nd_skd_dir               = OceanRoute.to_bound_code_value(row[9])
  n3rd_lane                  = ServiceLane.find_by(code: row[10])
  n3rd_pol                   = Location.find_by(code: row[11])
  n3rd_pod                   = Location.find_by(code: row[12])
  n3rd_skd_dir               = OceanRoute.to_bound_code_value(row[13])
  val                        = OceanRoute.new(origin_location: origin_location, destination_location: destination_location, n1st_lane: n1st_lane, n1st_pol: n1st_pol, n1st_pod: n1st_pod, n1st_skd_dir: n1st_skd_dir, n2nd_lane: n2nd_lane, n2nd_pol: n2nd_pol, n2nd_pod: n2nd_pod, n2nd_skd_dir: n2nd_skd_dir, n3rd_lane: n3rd_lane, n3rd_pol: n3rd_pol, n3rd_pod: n3rd_pod, n3rd_skd_dir: n3rd_skd_dir)
  val.created_by                 = User.find_by(name: 'yo')
  val.updated_by                 = User.find_by(name: 'yo')
  ocean_routes << val
end
OceanRoute.import ocean_routes_columns, ocean_routes, validate: false

I would try a couple of improvements.

Don't call Location.find_by(code: row[0]) on each iteration. Load all the locations once at the beginning with locations = Location.pluck(:id, :code) and then use origin_location_id = locations.find { |l| l[1] == row[0] }.
Do the same with the other attributes where you need a location: destination_location_id, n1st_pol_id, etc...
Do the same with OceanRoute and ServiceLane
Load the user user = User.find_by(name: 'yo') before the cycle and use it in every iteration: val.created_by = user
Use the fastest implementation of ActiveRecordImport: https://github.com/zdennis/activerecord-import/wiki/Examples#import-using-columns-and-arrays

Do these steps one by one to improve your performance.


C like Cpp code to do strncpy and copy a string to a buffer which is not long enough The code works, but I guess it could be much better.

/*
DESCRIPTION
The strncpy() function is similar, except that at most n bytes of src
are copied. Warning: If there is no null byte among the first n bytes
of src, the string placed in dest will not be null-terminated.
NOTE
If the length of src is less than n, strncpy() pads the remainder of
dest with null bytes.
*/

char *strncpy(char *dest, const char *src, size_t n)
{
    size_t i = 0;
    char *d = dest;
    while ((i++ < n) && (*d++ = *src++));
    while (i++ < n)
    {
        *d++ = 0;
    }
    return dest;
}

/*
DESCRIPTION
The stlcpy() function copies the string pointed to by src.
to the buffer pointed to by dest.
The strings may not overlap.
NOTE
The destination string dest may *NOT* be
large enough to receive the copy, then the truncation happpens.
size_d is the size of destination buffer.
*/

char *strlcpy(char *dest, const char *src, size_t size_d)
{
    size_t i = 0;
    char * d = dest;
    while ((i < size_d) && (*d++ = *src++)) i++;
    if (i < size_d)//everything copied.
    {
        if (i < size_d -1)
        {
            *d = 0;
        }
        else
        {
            //truncating the last char.
            dest[i-1] = 0;
        }
    }
    else if (i == size_d)
    {
        //truncating the src, size_d - 1 bytes copied.
        dest[i-1] = 0;
    }
    else
    {
        //i > size_d, impossible.
    }
    return dest;
}

Since you have written it for practice (as indicated in your comment), and strncpy has been implemented in various platforms in rather similar manner, I assume that you want comment on the function behavior itself and not specifically whether it conforms to the strncpy POSIX function.

I am aware that the second function strlcpy handles many of the criticisms below, but there is no indication in your question that you have considered these.

My first problem with the strncpy is that it allows no indication of failure or indication of any behavior even though there is a possibility of a few different behaviors that the user may be interested in.

The first n characters may not contain a \0
Did we truncate at n? or was the src small enough?
How many characters were copied?

We are using the return value by returning the dest even though this information is no different from the parameter that was passed in.

Secondly, if the buffer was small enough, the function is zeroing out the remainder of the memory. This seems to be a wasteful effort. Perhaps it is enough to just null terminate the string?

int strncpy(char *dest, const char *src, size_t n) {
    size_t i = 0;
    while ((i++ < n) && (*dest++ = *src++));
    if (i < n) {
        *dest = 0;
        return i;
    }
    return -1;
}


Dice simulation and counting pairs This is my first time putting a code together for Java based on my own so i was hoping if you someone would be kind enough to review and provide feedback or constructive criticism on this code.

The goal is to roll 2 dice and 10k times. add pairs and display their frequency.

The code runs fine but maybe I am overlooking some logical error or a better way to do this.

/**
 * Use the Random Number generator to write a java application that simulates a pair of dice.
 * Throwing the pair of dice 10,000 times- Add the values for the pair
 * Print the frequency at the end of 10,000 runs
 * @author
 *
 */

import java.util.Random;

public class diceSimulation {

    public static void main(String[] args) {
        /**
         * Declare variables and store dice max roll in Thousand
         */
        final int THOUSAND = 10000;
        int counter, dice1, dice2;

        //initiate an array with elements to keep count
        int [] diceValue = new int [7];

        // display welcome message. no other purpose but trial
        welcome ();

        // create new instance of random
        Random rollDice = new Random();

        /**
         * Set counter to start from 1 and go till desired constant number
         * Rolling two separate dices and storing values in dice1 & dice 2 respectively
         */
        for (counter=1; counter<=THOUSAND;counter++){

            dice1=rollDice.nextInt(6) +  1;
            dice2=rollDice.nextInt(6) + 1;

            // If statement to check if values are the same or not
            if (dice1==dice2){
                // IF values are same then go into for loop and store value in array element
                for (int i=1; i<=6; i++){
                    if (dice1 == i && dice2 == i){
                        // add value for the number displayed into the array
                        diceValue [i] += 1;
                    }
                }
            }

        }
        // Display results totals of paired rolls
        for (int a=1; a<diceValue.length; a++){
            System.out.println(" You rolled set of " + a + " " + diceValue[a] + " times");
        }

    }

    public static void welcome () {
        System.out.println("welcome to dice world!");
    }

}

Minor things

The java naming standard for classes is CamelCase (DiceSimulation instead of diceSimulation)

A constant of THOUSAND is not helpful, especially when the value is 10,000. A more meaningful name, say ROLLCOUNT, would be better.

Similarly, diceValue is not clear.  It contains the counts for each double occurence, so perhaps, doubleCounts - note the plural for an array indicating that it contains multiple values.

We have many dice but each one is a die -> die1 and die2.

Comments that just restate the obvious code can be omitted
e.g.

/**
* Declare variables and store dice max roll in Thousand
*/

// create new instance of random

// If statement to check if values are the same or not

Coding

We are working with 6 sided dice so we only need 6 entries in the array to store the double counts not 7 - it is more standard to adjust the index when outputting (e.g., add 1) than to make the arrays larger and use then as 1 based arrays..

We can easily store the number of sides in a variable (constant) to make consistency easier - a named value instead of a literal 6 in multiple places - and even allow us to use this for dice with other numbers of sides if we desired.

We can put the 'roll' functionality into a function so that to reduce duplication and allow us to change the implementation if desired (say for testing)

We check if dice1 == dice2 but then in the loop check if both dice1 == i && dice2 == i - if dice1 == dice2 then the second check can never fail.

If we index into the array, we don't need the loop

if (die1==die2){
    doubleCounts[die1-1]++;
}

If have sized the array to the number of elements then we should loop from 0 not 1 (and single character variable names, even for loops, are generally frowned upon)

System.out.format is a nicer way - simpler to use than the concatenation - for writing the output line

import java.util.*;

public class DiceSimulation {

    private static Random rollDice = new Random();

    public static void main(String[] args) {

        final int ROLLCOUNT = 10000;
        final int SIDES = 6;
        int counter, die1, die2;

        //an array with elements to keep count
        int [] doubleCounts = new int [SIDES];

        // display welcome message. no other purpose but trial
        welcome ();

        /**
         * Set counter to start from 1 and go till desired constant number
         * Rolling two separate dices and storing values in dice1 & dice 2       respectively
         */
        for (counter=1; counter<=ROLLCOUNT;counter++){

            die1=roll(SIDES);
            die2=roll(SIDES);

            if (die1==die2){
                doubleCounts[die1-1]++;
            }

        }

        // Display results totals of paired rolls
        for (int idx=0; idx<doubleCounts.length; idx++){
            System.out.format(" You rolled set of %d %d times\n",(idx+1), doubleCounts[idx]);
        }
    }
    private static int roll(int sides){
        return rollDice.nextInt(sides) +  1;
    }
    public static void welcome () {
        System.out.println("welcome to dice world!");
    }
}


Implementing a queue in Java for a technical interview This code is based off the Queue implementation in Chapter 3 of Cracking The Coding Interview. I modified the code to make it compile and give me the correct output. I'd appreciate any feedback on code style and correctness, assuming that I write this code in a technical interview.

public class Queue {
    Node first;
    Node last;

    public Queue(Node f, Node l) {
        first = f;
        last = l;
        first.next = last;
    }

    public void enqueue(int data) {
        if( first == null ) {
            last = new Node(data);
            first = last;
        } else {
            last.next = new Node(data);
            last = last.next;
        }
    }

    public int dequeue() {
        if (first != null) {
            int item = first.data;
            first = first.next;
            return item;
        }
        else {
            return -1;
        }
    }

    public static void main(String[] args) {
        Queue queue = new Queue(new Node(2), new Node(3));
        queue.enqueue(4);
        queue.enqueue(5);

        System.out.println(queue.dequeue() == 2);
        System.out.println(queue.dequeue() == 3);
    }

    public static class Node {
        int data;
        Node next;

        public Node(int d) {
            data = d;
        }
    }
}

You don't have a parameterless constructor which forces each queue to be instantiated with 2 elements. This isn't userfriendly since I might very wel want to construct it first and then add 5 bazillion elements to it.

Your code already allows it though: technically first can never be null as it is, yet you check for it. You can add a constructor without a problem.

Right now you only accept integers; perhaps you could make it generic to allow for each type?

Node is public; I'd change that to private since the outside world should never use it anyway.

Your code doesn't work; executing it will return 2 and -1, not 3. This happens because you don't set the next property of the nodes that are passed in to the Queue(Node, Node) constructor.


Go Lang custom convert array into string with two separators This is my custom function to join strings from an array into one string.
We can provide normal separator and the last one. JoinStringsIntoArray(", ", " and ", words...)
func JoinStringsIntoArray(sep string, lastSep string, words ...string) string {
lastElementIndex := len(words) - 1
var joinedString string

for index, word := range words[0:lastElementIndex] {
    if index == lastElementIndex-1 {
        sep = lastSep
    }
    joinedString += word + sep
}

joinedString += words[lastElementIndex]

    return joinedString
}

Test for this function
func TestStringsJoinsWithCommas(t *testing.T) {
    var words []string
    words = append(words, "one", "two", "three")

    expectedResult := "one, two and three"
    result := JoinStringsIntoArray(", ", " and ", words...)

    if expectedResult != result {
        t.Errorf("Strings joiner is broken, it should be '%v', we got '%v'", expectedResult, result)
    }
}

What do you think about this, can this solution be improved? The function works fine but I'm not so sure it's written well.

The Go standard library strings package has a Join function:
func Join(elems []string, sep string) string

Your function is a simple extension of strings.Join:
func JoinLast(elems []string, sep, lastSep string) string

However, for no obvious reason, you chose a different function parameter signature:
func JoinLast(sep string, lastSep string, words ...string) string

We need a comment to explain why this isn't an arbitrary or idiosyncratic decision.

Your testing is inadequate. Check boundary conditions. Your function fails on an empty words list.
words = []string{}

You have not provided any testing benchmarks. For example,
BenchmarkJoin-4   5534918  207.9 ns/op   104 B/op  5 allocs/op

func JoinStringsIntoArray(sep string, lastSep string, words ...string) string {
    lastElementIndex := len(words) - 1
    var joinedString string

    for index, word := range words[0:lastElementIndex] {
        if index == lastElementIndex-1 {
            sep = lastSep
        }
        joinedString += word + sep
    }

    joinedString += words[lastElementIndex]

    return joinedString
}

func BenchmarkJoin(b *testing.B) {
    words := []string{"one", "two", "three", "four", "five"}
    b.ReportAllocs()
    b.ResetTimer()
    for N := 0; N < b.N; N++ {
        JoinStringsIntoArray(", ", " and ", words...)
    }
}

You can do better. For example, minimize allocation,
BenchmarkJoin-4  16612879   70.95 ns/op   32 B/op  1 allocs/op

func JoinLast(elems []string, sep, lastSep string) string {
    var join strings.Builder
    joinCap := 0
    for i := 0; i < len(elems); i++ {
        joinCap += len(elems[i])
    }
    if len(elems) > 1 {
        joinCap += len(lastSep)
        if len(elems) > 2 {
            joinCap += (len(elems) - 2) * len(sep)
        }
    }
    join.Grow(joinCap)

    last := len(elems) - 1 - 1
    for i, elem := range elems {
        if i >= last {
            if i == last {
                sep = lastSep
            } else {
                sep = ""
            }
        }
        join.WriteString(elem)
        join.WriteString(sep)
    }

    return join.String()
}

func BenchmarkJoin(b *testing.B) {
    words := []string{"one", "two", "three", "four", "five"}
    b.ReportAllocs()
    b.ResetTimer()
    for N := 0; N < b.N; N++ {
        JoinLast(words, ", ", " and ")
    }
}


"Is a number prime?" I've come up with a very efficient way of finding if a number is prime, using TI-BASIC for use with TI-83/84/+/SE calculators. I am trying to optimize it however possible.

:Input "NUMBER: ",A
:If A<2 or fPart(A
:Then
:Disp "INVALID INPUT"
:Stop
:End
:0→B
:2→I
:While I<A and not(B
:If 0=fPart(A/I
:1→B
:I+2→I
:If I=4
:3→I
:End
:If B
:Disp "NOT"
:Disp "PRIME"

A few notes:

Lines 2-6 are for validating input, and don't affect the effectiveness of the program.
The closing "s on line 4 and the last two lines are not necessary, but I added them so the syntax would highlight nicely here.
Lines 12-14 are for speeding up the loop doubly; instead of incrementing by 1 each time, it is by 2, with the If to offset the 4 to 3 on the first run.
The not(B was very efficient, to end the loop whenever a match to identify the number as a composite number is found.

I do not speak TI-BASIC, so I cannot review the style, coding conventions etc. But there are two possible optimizations:

If a number A is composite then it must have a factor that is less than or
equal to sqrt(A). So you can replace

:While I<A and not(B

with (and I am just guessing the syntax):

:sqrt(A) -> S
:While I<=S and not(B

This  reduces the number of trial divisions substantially  if the input is a prime number.
Check the divisibility by 2 first, and then loop just over the odd numbers
I = 3, 5, 7, .... This saves you from checking

:If I=4
:3->I

in each loop iteration.


A script for a photo gallery/slideshow I'm a beginner in JavaScript.  I wrote this code so the main image get set on whatever thumbnail image the user clicks.

And if the user didn't click on any image the main photo get changed every 10 seconds.

Here's my code (I feel like I'm doing some unnecessary work here in the timers but I can't figure it out):

HTML:

<!DOCTYPE html>

    <title>Photo Gallery</title>
    <link rel="stylesheet" href="style.css" />

JavaScript:

window.onload = function(){
    var mainImage = document.getElementById("mainImage").getElementsByTagName("img")[0],
        thumbnailImages = document.getElementById("thumbnails").getElementsByTagName("img"),
        j = 0;

    var startTimer = function(){
        stillTimer = setInterval(function(){
                j = (j < 3) ? ++j : 0 ;
                mainImage.src = thumbnailImages[j].src;

            }, 10000);
    }

    startTimer();

    for(i = 0 ; i < thumbnailImages.length ; i++){
        thumbnailImages[i].addEventListener("click", function(evt){
            clearInterval(stillTimer);
            mainImage.src = evt.target.src;
            ++j;
            startTimer();
        });
    };
}

CSS:

body {
    margin: auto;
    width: 800px;
    height: 800px;
}

#mainContainer {
    width: 760px;
    height: 800px;
}

#mainImage {
    margin: auto;
    height: 560px;
}

#mainImage img {
    margin: 10px;
    width: 740px;
    height: 540px;
}

#thumbnails {
    padding-top: 10px;
    height: 205px;
}

.imgThumbnails {
    margin-left: 6px;
    height: 186px;
    width: 180px;
    display: inline;
    hover:
}

.imgThumbnails:hover {
    cursor: pointer;
}

I'm not a JS guy, so I focus on your HTML and CSS.

HTML

You're using the HTML5 doctype, so you can safely ommit the type-attribute on the script-tag
Add the viewport meta tag to the head-area to ensure correct viewport behavior on mobile devices:<meta name="viewport" content="width=device-width, initial-scale=1.0">(Note: I left out the self-closing / for the meta tag as well)
Avoid using ID's excessively; Things like caption and even a gallery itself are likely to occur multiple times on the same page – Classes are more appropriate there

CSS:

You have an error in the .imgThumbnails -rule, where you use a hover-property; There is no such property!
Side note: There is also no semi-colon. Although one can ommit the last semi-colon in a list of property-declarations, I highly encourage anyone not to do this. It just leads to unnecessary trouble
You're using a lot of fixed widths and heights. This is very restrictive and should be avoided.

Before I get deeper in to making suggestions on how to improve your code further, it would be great to have some kind of demo. This doesn't need to be a working example of your gallery, you can use placeholders instead of images.


Ignoring the effect of a space when applying encryption I am writing an encryption program that will encrypt the message "STOP POLLUTION" with the encryption function f(p) = (3p + 7) mod 26 where p is given letter.

Here is my code for doing so (tested it works):

    static final int SPACE = -33;
    static int[] convert(String s) {
        char[] array = s.toCharArray();
        int[] distances = new int[s.length()];
        for(int c=0;c<distances.length; c++) {
            distances[c] = array[c] - 'A';
        }
        return distances;
    }
    static String convertBack(int[] distances) {
        String result = "";
        for(int c=0;c<distances.length;c++) {
            char distance = (char) ('A' + distances[c]);
            result += distance;
        }
        return result;
    }
    static void applyShift(int[] distances, int a, int b, int mod) {
        for(int c=0;c<distances.length;c++) {
            if(distances[c]!=SPACE){
                distances[c] = (a*distances[c] + b) % mod;
            }
        }
    }

My question is regarding the space in "STOP POLLUTION". I don't really care about that space in converting the String into an array of distances from zero or converting that array back to a String but I do care about the space when applying the encryption algorithm. I want to leave the space as it is because if I change it, I won't be able to write out the space in the encrypted form.

I do this by checking if the current letter I am processing is a space. To me, this is inefficient because there is only one space. Is there a better way to do this (some way to ignore the checking of the space) but still have the space stay the same?

Before I answer your question, I'm gonna give a more general review:

Spaces are good things. They help readability a lot. For example, this:

for(int c=0;c<distances.length;c++) {

should be this:

for (int c = 0; c < distances.length; c++) {

The rules I use are complicated, but it boils down to a space on either side of a keyword, binary operator, or curly brace.
Put a newline between your methods. After the closing brace of one, you should have a blank line, then any Javadocs for the next method, then the next method itself.
convert should probably be a private static, and the rest should be public static. It's very rare that you actually want to use package-private things; more often than not, the other three permissions cover every case.
Use advanced for loops when the only thing you use the index for is accessing a single array.
Instead of repeated + on Strings, use a StringBuilder and .append. For example, this:

String result = "";
for (int c : distances) {
    char distance = (char) ('A' + c);
    result += distance;
}
return result;

becomes this:

StringBuilder result = new StringBuilder(distances.length);
for (int c : distances) {
    char distance = (char) ('A' + c);
    result.append(distance);
}
return result.toString();

It's more performant in terms of speed and memory.
Use better variable names than array, c, etc. Those tell us nothing about what they really are.

Now, on to your question: Yes, you can do this. In fact, there are a couple of ways to do it. You've gone for the most obvious -- and, for only one or two letters, arguably best -- option. However, as you start to have more and more letters you want to ignore, it becomes a hassle to add them all to the if.

One thing you could do is declare a static final char[] forbiddenLetters and, each iteration, check if the letter (or, more accurately, 'A' + convertedLetter) is contained in forbiddenLetters, skipping over it if so.

Another option is, instead of using a blacklist, use a whitelist: Have a static final char[] validLetters and use it much the same way, but applying the encryption if the letter is in the list or not. This works better for your case, as I think you'll probably have a good deal more forbidden letters than valid ones. If you just want capital letters, you could also apply the same concept by checking that for each (converted) letter \$l\in\mathbb{Z}\$, \$l\in\left[0,25\right]\$

I'm gonna leave the implementation of that up to you, though. Both ought to be fairly easy -- especially the second suggestion in the latter -- to translate to Java.


Makefile and directory structure I am looking for improvements for this basic makefile and directory structure. I know this is overkill and could be compiled with one line in a flat directory structure, but I want to use this as a learning exercise.

CC=g++
CFLAGS=-Wl,--no-as-needed -std=c++11
LIBS=-lpthread
SRC=$(PWD)/src
BUILDDIR=$(PWD)/build
OUTDIR=$(BUILDDIR)/bin
TEMPDIR=$(BUILDDIR)/tmp
MKDIR_P = mkdir -p

.PHONY: directories clean run

all: directories $(OUTDIR)/terminal

$(OUTDIR)/terminal: $(TEMPDIR)/main.o $(TEMPDIR)/terminal.o
    $(CC) $(CFLAGS) $(TEMPDIR)/terminal.o $(TEMPDIR)/main.o $(LIBS) -o $(OUTDIR)/terminal

$(TEMPDIR)/main.o: $(SRC)/main.cpp $(SRC)/terminal.hpp
    $(CC) -c $(CFLAGS) $(SRC)/main.cpp -o $(TEMPDIR)/main.o

$(TEMPDIR)/terminal.o: $(SRC)/terminal.cpp $(SRC)/terminal.hpp
    $(CC) -c $(CFLAGS) $(SRC)/terminal.cpp -o $(TEMPDIR)/terminal.o

directories: ${OUTDIR} $(TEMPDIR)

$(OUTDIR):
    ${MKDIR_P} $(OUTDIR)

$(TEMPDIR):
    ${MKDIR_P} $(TEMPDIR)

clean:
    rm -rf $(BUILDDIR)

run:
    $(OUTDIR)/terminal

Stem rules:

Spelling out compilation rule for each object file is tedious and error prone.

$(TEMPDIR)/%.o: $(SRC)/%.c
    $(CXX) $(CFLAGS) -c $< -o $@

takes care about all of them.
Autodependencies:

Spelling out dependencies for each object file is tedious and error prone. gcc can do it for you:

$(TEMPDIR)/main.d: $(SRC)/main.c
    $(CXX) $(CFLAGS) -MM -MT $< -o $@

-include $(TEMPDIR)/main.d

Of course you we don't want to spell it out for each source file, which naturally leads us to the next step.
Use macros. Shall you add more source files, only FILES needs to be modified.

FILES = main.c terminal.c
SOURCES = $(patsubst %,$(SRC)/%,$(FILES))
OBJECTS = $(patsubst %.c,$(TEMPDIR)/%.o,$(FILES))
DEPS    = $(patsubst %.c.$(TEMPDIR)/%.d,$(FILES))

$(TEMPDIR)/%.o: $(SRC)/%.c
    $(CXX) $(CFLAGS) -c $< -o $@

$(TEMPDIR)/%.d: $(SRC)/%.c
    $(CXX) $(CFLAGS) -MM -MT $< -o $@

-include $(DEPS)

$(TARGET): $(OBJECTS)
    $(CXX) $(LDFLAGS) $(OBJECTS) -o $(TARGET)