Converting Celsius and Fahrenheit in C++ I've made my first little program written in C++ and was wondering if anyone could evaluate it and suggest some improvements. No copy paste. I was wondering if I can do something with my little knowledge and came up with the simple idea of converting Celsius and Fahrenheit.

I don't know why, but when using 9/5, it gives a different output than when using 1.8 or 9.0/5.0.

#include <iostream>
using namespace std;

// My very first code written by myself

  char temperature; // what are we converting ( celsius or farenheit into another )
  float value; // value that is being converted ( -10, 0, 10, 20.5 etc. )

int main()
{
    cout << "Welcome" << "\n" << "This little program converts Celsius to Fahrenheit and vice versa" << endl;
    cout << "Enter c for Celsius, f for Fahrenheit or p for proof" << endl;
    cin >> temperature; // storing input which accepts only c, f, p
    switch (temperature) { // checking input for one of 3 accepted options, else using default
        case 'c' : {
            cout << "Converting Celsius to Fahrenheit" << "\n" << "Enter value" << endl;
            cin >> value; // storing the value of temperature
            float farenheit = value*1.8+32; // converting celsius to fahrenheit using the formula
            cout << value << " Celsius in Fahrenheit is: " << farenheit << endl;
        }
            break;
        case 'f' : {
            cout << "Converting Fahrenheit to Celsius" << "\n" << "Enter value" << endl;
            cin >> value; // storing the value of temperature
            float celsius = (value-32)/1.8; // converting fahrenheit to celsius using the formula
            cout << value << " Fahrenheit in Celsius is: " << celsius << endl;
        }
            break;
        case 'p' : {
            cout << "Proof that program is running correctly, here is a list of -20 to 20 Celsius converted to Fahrenheit" << endl;
            for (int i=-20; i<21; i++) { // using a for loop to print Celsius to Fahrenheit values from -20 to 20
                cout << i << " = " << i*1.8+32 << endl; // output Celsius = Fahrenheit
            }
        }
            break;
        default:
            cout << "Invalid input! Use c, f or p" << endl; // any input except the 3 accepted options will result in this default output
    }
    cout << "Made by blackborg";
    return 0;
}

You might as well break the habit of using using namespace std. It involves a little more additional code, but it'll be helpful in the future, especially when using header files.
You don't need those global variables outside of main(). You should really avoid global variables unless absolutely necessary since they can be changed from any scope and you may not even know it. Just put those two variables inside if main() before their use.
farenheit is spelled incorrectly; it should be fahrenheit.
What if the user uses an uppercase 'C' or 'F' instead? You should account for those as well:

case 'c':
     'C': {

case 'f':
     'F': {

It may be more readable to multiply by both 9.0/5.0 and 5.0/9.0 respectively instead of 1.8 since that's most common with these conversion formulas. Make sure not to use integers for this, otherwise the division will truncate towards 0 (in this case, you'll end up with 1 instead of 1.8 and 0 instead of 0.555556).
You may find it better to use double instead of float in most cases and because the former has more precision.
For the next step up, I would recommend adding additional functions since main() shouldn't have to do all of the work.

You can simply acquire value from main(), pass it to the applicable function, then return the result and display it. It'll also allow you to avoid duplicating code in the switch, such as having multiple inputs for value, which is very nice and will be essential in every other program you create. I'll let you come up with that yourself.
If you want to make your program more useful, you can also add Kelvins.
You should probably put that tagline cout in a comment instead, otherwise it may be annoying to see it come up each time when only a result is expected to be printed.
You don't really need return 0 at the end of main() since success is already guaranteed at that point, though it won't hurt anything either.


python segment tree - count of smaller numbers after self I am trying to solve this question https://leetcode.com/problems/count-of-smaller-numbers-after-self/

  You are given an integer array nums and you have to return a new
  counts array. The counts array has the property where counts[i] is the
  number of smaller elements to the right of nums[i].

My code:

import bisect

from functools import lru_cache

def merge(a, b):
    i, j = 0, 0
    res = []
    while i < len(a) and j < len(b):
        if a[i] < b[j]:
            res.append(a[i])
            i += 1
        else:
            res.append(b[j])
            j += 1
    while i < len(a):
        res.append(a[i])
        i += 1
    while j < len(b):
        res.append(b[j])
        j += 1
    return res

class SegmentTree:
    def __init__(self, nums):
        self.nums = nums
        self.tree = [0] * (len(nums)*4)
        if len(nums)>0:
            self._build()

    def _build(self):
        def build(l, r, index):
            if l == r:
                self.tree[index] = [self.nums[l]]
                return self.tree[index]
            else:
                mid = (l+r)//2
                left_set = build(l, mid, index*2)
                right_set = build(mid+1, r, index*2+1)
                m = merge(left_set, right_set)
                self.tree[index] = m
                return m
        build(0, len(self.nums)-1, 1)

    def get_range(self, l, r):

        @lru_cache(maxsize=None)
        def get_range(left_boundary, right_boudndary, index, l, r):
            if l > r:
                return []
            if left_boundary == right_boudndary:
                return self.tree[index]
            if left_boundary == l and right_boudndary == r:
                return self.tree[index]
            else:
                mid = (left_boundary + right_boudndary)//2
                left = get_range(left_boundary, mid, index*2, l, min(r, mid))
                right = get_range(mid+1, right_boudndary,
                                index*2+1, max(l, mid+1), r)
                return merge(left, right)

        return get_range(0, len(self.nums)-1, 1, l, r)

class Solution:
    def countSmaller(self, nums: List[int]) -> List[int]:
        s = SegmentTree(nums)
        result = []
        for i in range(len(nums)):
            res  = s.get_range(i+1, len(nums)-1)
            ans = bisect.bisect(res, nums[i]-1)
            result.append(ans)
        return result

Sadly this times out on the last case :(

How do I speed this up?

(To be clear: I am looking for a faster solution with segment trees, not fenwick tree)

So, the time complexity of the solution posted in the question is O(n^2 log(n)).

Answering each query takes n log(n), and we have n queries in total.

We don't necessarily need to merge the left and right subtrees to find the inversion count; given that the sublists are sorted, we can exploit binary search.

import bisect

from functools import lru_cache

def merge(left, right):
    res = []
    i, j = 0, 0
    while i<len(left) and j<len(right):
        if left[i]<right[j]:
            res.append(left[i])
            i+=1
        else:
            res.append(right[j])
            j+=1

    while i<len(left):
        res.append(left[i])
        i+=1
    while j<len(right):
        res.append(right[j])
        j+=1
    return res

class SegmentTree:
    def __init__(self, nums):
        self.nums = nums
        self.tree = {}
        if len(nums)>0:
            self._build(0, len(nums)-1, 1)
    def _build(self, l, r, index):
        if l==r:
            self.tree[index] = [self.nums[r]]
            return self.tree[index]
        else:
            mid = (l+r)//2
            left= self._build(l, mid, index*2)
            right = self._build(mid+1, r, index*2+1)
            self.tree[index] = merge(left, right)
            return self.tree[index]

    def get_range(self, l,r, target):
        def get_range(left_boundary, right_boundary, l, r, index):
            if l>r:
                return 0
            if left_boundary == right_boundary or (left_boundary == l and right_boundary==r):
                return bisect.bisect(self.tree[index], target)
            else:
                mid = (left_boundary+ right_boundary)//2
                left = get_range(left_boundary, mid, l, min(r, mid), index*2)
                right = get_range(mid+1, right_boundary, max(l, mid+1), r, index*2+1)
                return left+right
        return get_range(0, len(self.nums)-1, l, r, 1)

class Solution:
    def countSmaller(self, nums: List[int]) -> List[int]:
        s = SegmentTree(nums)
        result = []
        for i in range(len(nums)):
            res = s.get_range(i+1, len(nums)-1, nums[i]-1)
            result.append(res)
        return result


Self-written Mutex for 2+ Threads I have written the following code, and so far in all my tests it seems as if I have written a working Mutex for my 4 Threads, but I would like to get someone else's opinion on the validity of my solution.

typedef struct Mutex{
    int turn;
    int * waiting;
    int num_processes;
} Mutex;

void enterLock(Mutex * lock, int id){
    int i;
    for(i = 0; i < lock->num_processes; i++){
        lock->waiting[id] = 1;
        if (i != id && lock->waiting[i])
            i = -1;
        lock->waiting[id] = 0;
    }
    printf("ID %d Entered\n",id);
}

void leaveLock(Mutex * lock, int id){
    printf("ID %d Left\n",id);
    lock->waiting[id] = 0;
}

Broken
If you call enterLock() with id 0 followed by enterLock() with id 1 (on another thread perhaps), both calls will succeed.  Since your function doesn't change the state of the mutex, it isn't surprising.  Perhaps you meant to leave lock->waiting[id] at 1 instead of 0?  Note, even if you did that, it wouldn't be enough to fix the mutex.
Also, what is the turn variable used for?
Spinlock, not mutex
Technically, what you are doing is a spinlock.  A real mutex should efficiently wait for its turn instead of spinning in a loop.


Precision sum of doubles Summing doubles can cause a loss of precision.
Summing integers, however, doesn't. (Assuming no overflow.)

Here's a high precision sum function I wrote:

#include <iostream>
#include <vector>
#include <cmath>
#define BASE 10

bool is_int(double n) {
  double intpart;
  return std::modf(n, &intpart) == 0.0;
}

double sum(std::vector<double> nums) {
  int n = nums.size();

  int maxepower = 1;
  for (int i = 0; i < n; ++i) {
    while(!is_int(nums[i] * pow(BASE, maxepower))) { maxepower++; }
  }

  int pieces[n];
  for (int i = 0; i < n; ++i) {
    pieces[i] = nums[i] * pow(BASE, maxepower);
  }

  int sum = 0;
  for (int i = 0; i < n; ++i) {
    sum += pieces[i];
  }

  return sum / pow(BASE, maxepower);
}

I'm new to C++ so extra feedback on idioms would be helpful.

EDIT:
Example:

int main() {
  std::cout << 0.1 + 0.2 - 0.3 << std::endl;
  std::cout << sum({0.1, 0.2, -0.3}) << std::endl;
}

gives

5.55112e-17
0

It has a bug: very small numbers are completely ignored:

sum({0.1, 0.0000001}) -> 0.1

If technical limitations do not allow you to change this behaviour, you should:

Document it in a comment as a know limitation
Put a condition in the loop if x < min_tolerance {fail}, to adhere to fail fast fail loudly, instead of silently failing.


Writing binding providers in CoffeeScript? In his KnockoutJs 1.3 beta post, Steve Sanderson has an example of using Binding providers ("5. Binding providers (and hence external bindings)") where he makes a comment in the JSFiddle JavaScript tab that says "// This syntax would be way more succinct with CoffeeScript".

How would this actually be accomplished?

// This syntax would be way more succinct with CoffeeScript
ko.bindingConventions.conventions(".person-editor", {
    ".person-editor"  : { 'with': myViewModel.person },
    ".first-name"     : function(person) { return { value: person.firstName } },
    ".last-name"      : function(person) { return { value: person.lastName } }
});

ko.bindingConventions.conventions("#weather-list", {
    "#weather-list"    : { 'with': myViewModel.weather },
    ".cities-list"     : function(weather) { return { foreach: weather.cities } },
    ".city"            : function(item) { return { text: item.city } },
    ".temp"            : function(item) { return { text: item.temperature } },
    ".add-city"        : { click: function() { this.addItem() } }
});

ko.bindingConvenrions.conventions ".person-editor"
   ".person-editor"  :
      'with'   :  myViewModel.person
   ".first-name"     : ( person ) ->
                        value : person.firstName
   ".last-name"      : ( person ) ->
                        value : person.lastName


Hashtag Creator: Find frequent words and display their occurrences I've done a Python script to find frequent keywords from raw texts and display them along with source sentences and texts. But, having not exercised my Python for an age, I'm sure the code is full of smell. Any pointers would be warmly appreciated.

import os
import nltk
import string
import json
from datetime import datetime
from nltk.tokenize import word_tokenize
from nltk.corpus import stopwords

class Word:
    """Stores data relating to individual words."""

    def __init__(self, spelling):
        self.spelling = spelling
        self.sentences = []
        self.frequency = 0
        self.found_in = []

    def add_sentences(self, *sentences):
        [self.sentences.append(s) for s in sentences]

    def to_json(self):
        structure = {self.spelling: {"Sentences": self.sentences, "Documents": self.found_in}}
        return structure

class Hashtagger:
    """Extracts frequent words from text, and the sentences in which they are found."""

    def __init__(self, raw_text):
        self.raw_text = raw_text
        self.words = []
        self.sentences = []
        self.stopword_list = stopwords.words('english') # words to not process
        self.lemmatizer = nltk.WordNetLemmatizer() # groups inflections to single word
        self.process_text()

    def process_text(self):
        self.process_words(self.raw_text)
        self.deconstruct_sentences(self.raw_text)
        self.allocate_sentences_to_all()

    def process_words(self, text):
        stripped_text = self.strip_punctuation(text)
        tokens = nltk.Text(word_tokenize(stripped_text))
        content = [self.lemmatizer.lemmatize(t.lower()) for t in tokens if t not in self.stopword_list]
        counts = {t: content.count(t) for t in content}

        for token, count in counts.items():
            word = Word(token)
            word.frequency = count
            self.words.append(word)

    def deconstruct_sentences(self, text):
        raw_sentences = nltk.sent_tokenize(text)
        self.sentences = [s.strip(string.punctuation) + '.' for s in raw_sentences]

    def allocate_sentences_to(self, word):
        sentences_containing = [s for s in self.sentences if word.spelling in s.lower()]
        word.add_sentences(*sentences_containing)

    def allocate_sentences_to_all(self):
        [self.allocate_sentences_to(key) for key in self.words]

    def strip_punctuation(self, text):
        for ch in string.punctuation:
            text = text.replace(ch, ' ')
        return text

class ResultsAccumulator:
    """Accumulates the results of many hashtaggers."""

    def __init__(self):
        self.word_tally = []

    def add_tally(self, hashtagger, file=None):
        for h_word in hashtagger.words:
            matched = False
            for word in self.word_tally:
                if word.spelling == h_word.spelling:
                    word.add_sentences(*h_word.sentences)
                    word.frequency += h_word.frequency
                    word.found_in.append(file)
                    matched = True
                    break
            if not matched:
                h_word.found_in.append(file)
                self.word_tally.append(h_word)

    def write_word_json(self):
        total_json = {}
        for word in self.word_tally:
            total_json.update(word.to_json())
        return total_json

def main():
    path = r'test_docs/'

    results = ResultsAccumulator()

    for file in os.listdir(path):
        with open(os.path.join(path, file), encoding='utf8') as d:
            raw_text = d.read()

        hashtagger = Hashtagger(raw_text)
        results.add_tally(hashtagger, file)

    final_tally = results.write_word_json()

    datestring = datetime.strftime(datetime.now(), '%Y-%m-%d-%H-%M-%S')
    with open('word_json_' + datestring + '.txt', 'w') as f:
        json.dump(final_tally, f, indent=4,
              ensure_ascii=False)

if __name__ == '__main__':
    main()

From what I can tell, the "weirdest" thing in your code is the following:
Avoid list comprehensions if all you want are side-effects
In your Word class you have the following:
def add_sentences(self, *sentences):
    [self.sentences.append(s) for s in sentences]

That is a weird function because generally list comprehensions are created because we care about the list they actually build, not because we want side-effects.
Therefore, if you wanted to create a loop to do the .append you do, you would write something like
def add_sentences(self, *sentences):
    for s in sentences:
        self.sentences.append(s)

Having said that, your sentences is an iterable so what you can do is
def add_sentences(self, *sentences):
    self.sentences.extend(sentences)

*args is nice but not always necessary
Now, if we keep dissecting this function, we see that it is often called with a single argument, which is an iterable (usually a list), so while the *args pattern is nice, it is useless if everything you are going to do is call the function with
word.add_sentences(*iterable)

The *args syntax is useful for when you don't know how many arguments you are getting and it makes sense to call the function with many arguments.
For your use case, I don't think we will see things like
the = Word("the")
the.add_sentences("The cat is nice", "I love the Python language")

This means that you can very well define your code to be
def add_sentences(self, sentences):
    self.sentences.extend(sentences)

And later you call it with
word.add_sentences(list_of_sentences)


jQuery hide and show option value from a dropdown I have form with an "add field" button that does an Ajax call to add a new field.

Each field contains a list of options to choose from. When is public is Yes, I need to remove option value = "contacts" and "file upload" from the drop and when the "Add field" button is clicked. If the is public is no, I need to show the option value "contacts" and "file upload".

My code works, but I feel I can improve it.

publicFormValidation = function(){
  $('#public').change(function(){
    if(checkForm()){
      $('.field_type option[value="file_upload"]').hide();
      $('.field_type option[value="contacts"]').hide();
      $('.add_many_fields').on('click',function(){
        setTimeout(function(){
          $('.field_type option[value="file_upload"]').hide();
          $('.field_type option[value="contacts"]').hide();
        }, 500);
      });
    }else {
      var $contacts = $('.field_type option[value="contacts"]');
      var $file_uploads = $('.field_type option[value="file_upload"]');
      $contacts.show();
      $file_uploads.show();
        $('.add_many_fields').on('click',function(){
          setTimeout(function(){
            var $contacts = $('.field_type option[value="contacts"]');
            var $file_uploads = $('.field_type option[value="file_upload"]');
            $contacts.show();
            $file_uploads.show();
        }, 500);
      });
    };
  });
}

checkForm = function(){
  var public_val = $('#public').val();
  return (public_val == "true");
}

The reason I have a setTimeout function is because when I needed to create a delay from when click to hide onto the DOM.

The primary things I would recommend here are

avoid polluting the global namespace
avoid unintentionally creating globals
cache your DOM element lookups to increase performance and also decrease filesize

Here is the code reflecting these considerations:

// Wrap this in a self-executing, anonymous scope, or in a document.ready
// this way you can avoid polluting the global (window) namespace
(function(){

  // Cache this DOM element in the main scope because you look it up in both of your functions.
  var $public = $('#public');

  /* use "var" or publicFormValidation becomes a wild global */
  var publicFormValidation = function(){
    // Cache DOM elements that you're going to call multiple times, like ".add_many_fields"
    // DOM lookups are very expensive operations (depending on the size of the DOM)
    var $addManyFields = $('.add_many_fields'),
        $fileUpload = $('.field_type option[value="file_upload"]'),
        $contacts = $('.field_type option[value="contacts"]');

    $public.change(function(){
      if(checkForm()){
        $fileUpload.hide();
        $contacts.hide();
        $addManyFields.on('click',function(){
          setTimeout(function(){
            $fileUpload.hide();
            $contacts.hide();
          }, 500);
        });
      }else {
        $contacts.show();
        $file_uploads.show();
          $addManyFields.on('click',function(){
            setTimeout(function(){

              /* You definitely don't want to do this:

              var $contacts = $('.field_type option[value="contacts"]');
              var $file_uploads = $('.field_type option[value="file_upload"]');

              you're actually overwriting variables you've already defined in the parent scope - you
              already have these references, so no need to do another DOM lookup.
              */
              $contacts.show();
              $file_uploads.show();
          }, 500);
        });
      };
    });
  }

  /* Use var here as well to get rid of globals */
  var checkForm = function(){
    var public_val = $public.val();
    return (public_val == "true");
  }
}());


Converter for deserializing JSON I'm writing a custom JavaScript converter to deserialize some JSON. Here's a few lines I'm writing:

public override object Deserialize(IDictionary<string, object> dictionary, Type type,
      JavaScriptSerializer serializer)
  {
    int NoteID;
    NotesModel TheObject = new NotesModel();

    if (dictionary.ContainsKey("NoteID"))
    {
      if (int.TryParse(serializer.ConvertToType<string>(dictionary["NoteID"]),
          out NoteID))
      {
         TheObject.NoteID = serializer.ConvertToType<int>(dictionary["NoteID"]);
      }
    }

How can this be improved?


Reading large text file from network (SMB) I am facing performance issues while reading a large text file from the network. The same file is read locally without any performance problems:

int file_no_lines(char *file_path)
{
    FILE *fp = NULL;
    char line_read[200];
    int linecount = 0;
    fp = fopen(file_path, "r");
    while(1)
    {
     if (fgets(line_read, 160 ,fp) == NULL)
     {
        if(feof(fp))
        {
           fclose(fp);
           return linecount;
        }
        else
        {
            printf("Bad File Termination");
            fclose(fp);
            return -1 ;
        }
     }
     else
     {
        linecount++;
     }
    }
    fclose(fp);
    return linecount;
}
int file_read_lines(char *file_path)
{
    FILE *fp = NULL;
    char line_read[200];
    fp = fopen(file_path, "r");
    while(1)
    {
     if (fgets(line_read, 160 ,fp) == NULL)
     {
        if(feof(fp))
        {
           fclose(fp);
           return -1;
        }
        else
        {
            printf("Bad File Termination");
            fclose(fp);
            return -1 ;
        }
     }
     else
     {
        //update structure
     }
    }
    fclose(fp);
    return 1;
}
int _tmain(int argc, _TCHAR* argv[])
{

    int no_lines = file_no_lines("C:/sample.txt"); //count no. of lines
    //allocate structure for no. of lines
    file_read_lines("C:/sample.txt"); // fill allocated array
    return 0;
}

Steps:

Count the no. of valid lines using file_no_lines
Allocate for no. of lines
Update structure, file_read_lines

Reading just 160 bytes at a time could be bad for performance, especially if you are using the older SMB 1 protocol.  SMB 2.0 introduced pipelining, which allows the client to issue another request before the previous one has completed.  Therefore, with SMB 1, you would have to wait for each request to be finished — including network and disk latency — before making the next read.  If the read requests are too small, then performance could suffer badly.

What is a good buffer size?  Microsoft's KB 223140 suggests that somewhere between 4 kB and 60 kB could be ideal — in any case, much more than 160 bytes.


"specific case" object that stands for "any object" : is it a good idea ? I got tired of doing (more or less) this in my rails controllers :

class ThingsController < ApplicationController

  def index
    @category = Category.find( params[:category_id] ) if params[:category_id].present?
    scope = @category ? @category.things : Thing.scoped
    @things = scope.order( :title )
  end
end

...wich also leads to a lot of conditionals in the template if @category is absent.

I had this idea (a variation on the NullObject pattern) :

class Category < ActiveRecord::Base

  def self.one_or_any( id )
    where( id: id ).first || any_category
  end

  def self.any_category
    @any_category ||= begin
      new( title: "Any category" ) do |generic|
        generic.instance_eval{ def things ; Thing.scoped end }
        generic.readonly!
        generic.freeze
      end
    end
  end
end

Which would in turn allow me to do :

class ThingsController < ApplicationController
  def index
    @category = Category.one_or_any( params[:category_id] )
    @things   = @category.things.order( :title )
  end
end

... and to get rid of category-related conditionals in the template.

I Just wonder if this is a good idea, and if it has drawbacks i don't see yet. Thoughts ?

Note : Of course the actual implementation can be different (use a specific subclass of Category instead of extending an instance, for example - or even using a decorator).

I'd try to keep it as simple as possible. Instead of multiple in-line conditionals or a weird abstraction, I think a single full-fledged indented conditional makes things very clear. There are two scenarios, write two branches:

class ThingsController < ApplicationController
  def index
    if params[:category_id].present?
      @category = Category.find(params[:category_id])
      @things = @category.things
    else
      @category = nil
      @things = Thing.scoped
    end
  end
end


Calculate query coverage from BLAST output I have a BLAST output file and want to calculate query coverage, appending the query lengths as an additional column to the output.  Let's say I have

2 7 15

f=open('file.txt', 'r')
lines=f.readlines()
import re
for line in lines:
    new_list=re.split(r'\t+',line.strip())
    q_start=new_list[0]
    q_end=new_list[1]
    q_len=new_list[3]
    q_cov=((float(q_end)-float(q_start))/float(q_len))*100
    q_cov=round(q_cov,1)
    q_cov=str(q_cov)
    new_list.append(q_cov)
    r=open('results.txt', 'a')
    x='\t'.join(new_list)
    x=x+'\n'
    r.writelines(x)
f.close()
r.close()

One serious bug is that you open results.txt for each line of input.  It's almost always better to open files in a with block.  Then, you won't have to worry about closing your filehandles, even if the code exits abnormally.  The with block would have made your results.txt mistake obvious as well.

Since you want to treat your q_start, q_end, and q_len as numbers, I wouldn't even bother to assign their string representations to a variable.  Just convert them to a float as soon as possible.  Similarly, q_cov should be a float; I would just stringify it at the last moment.  I would also postpone rounding just for the purposes of formatting the output, preferring to preserve precision in q_cov itself.

Put your import statements at the beginning of the program.

import re

with open('file.txt') as input, open('results.txt', 'a') as output:
    for line in input.readlines():
        fields = re.split(r'\t+', line.strip())
        q_start, q_end, q_len = map(float, (fields[0], fields[1], fields[3]))
        q_cov = 100 * (q_end - q_start) / q_len
        print >>output, '\t'.join(fields + [str(round(q_cov, 1))])


Python: Making bowling algorithm more pythonic So I created an algorithm that keeps score of the amount of pins knocked down per frame. Since I am totally self taught, I am trying to see how I could make my code more efficient/more pythonic. Any advice would help. Also, is it proper to return a score or list into the init method as I did with the "overall" variable? Thanks.

class Bowling_player(object):

    def __init__(self, name, overall = 0):
        self.name = name
        self.overall = overall

    def frames(self):
        shot_one = int(input('how many pins on first shot:'))
        if shot_one == 10: # this is a strike
            shot_two_after_strike = int(input('how many pins on shot after strike:'))
            if shot_two_after_strike == 10: # you got another strike. continue the frame!
                shot_three_after_strike = int(input('how many pins on shot after two strikes:'))
                self.overall += (shot_three_after_strike + shot_two_after_strike + shot_one)
                return self.overall
            else:# you did not get another strike, frame over.
                self.overall += (shot_one + shot_two_after_strike)
                return self.overall

        elif shot_one < 10:
            shot_two = int(input('how many pins on second shot:'))
            shots = shot_one + shot_two
            if shots == 10: # this is a spare
                shot_three_after_spare = int(input('how many pins on shot after spare'))
                self.overall += (shots + shot_three_after_spare)
                return self.overall
            else: # you suck at bowling
                self.overall += shots
                return self.overall

craig = Bowling_player('craig')

craig.frames()

Overall, there is not much sense in using a class here - you are not using player's name and you are calculating the overall shots in the frames method and returning the overall value.

Some code style specific notes:

according to PEP8, the class name should be in CamelCase, call it BowlingPlayer
there is no need for parenthesis around the expressions, you can replace:

self.overall += (shot_three_after_strike + shot_two_after_strike + shot_one)

with:

self.overall += shot_three_after_strike + shot_two_after_strike + shot_one

there is some code duplication when you calculate the overall value, you can add up shot_two_after_strike and shot_one to the overall value and only if the score is 10, add the shot_three_after_strike:

self.overall += shot_two_after_strike + shot_one
if shot_two_after_strike == 10: # you got another strike. continue the frame!
    shot_three_after_strike = int(input('how many pins on shot after two strikes:'))
    self.overall += shot_three_after_strike
return self.overall

Same for the case when shot_one < 10.
according to PEP8 commenting guidelines, there has to be two spaces before the # and once space after
there is no need for spaces around the = for keyword function or method arguments (PEP8 reference)
put the main execution code to under if __name__ == '__main__': to avoid it being executed on import
let's define 10 as a constant to avoid hardcoding things
I'd also use something like a total_points variable name instead of a more questionable overall

Improved code:

POINTS_FOR_STRIKE = 10

class BowlingPlayer(object):
    def __init__(self, name, overall=0):
        self.name = name
        self.total_points = overall

    def frames(self):
        shot_one = int(input('how many pins on first shot:'))
        if shot_one == POINTS_FOR_STRIKE:  # this is a strike
            shot_two_after_strike = int(input('how many pins on shot after strike:'))
            self.total_points += shot_two_after_strike + shot_one

            if shot_two_after_strike == POINTS_FOR_STRIKE:  # you got another strike. continue the frame!
                shot_three_after_strike = int(input('how many pins on shot after two strikes:'))
                self.total_points += shot_three_after_strike
        elif shot_one < POINTS_FOR_STRIKE:
            shot_two = int(input('how many pins on second shot:'))
            shots = shot_one + shot_two
            self.total_points += shots

            if shots == POINTS_FOR_STRIKE:  # this is a spare
                shot_three_after_spare = int(input('how many pins on shot after spare'))
                self.total_points += shot_three_after_spare
        return self.total_points

if __name__ == '__main__':
    craig = BowlingPlayer('craig')
    print(craig.frames())


Setting up a development environment I'm looking for feedback on improving the structure of this code. Also, I could not find the command line executable for Chrome.

General feedback is also requested.

#
#
#
# Divider - configures bash, git, grunt, sublime and chrome
#
#
#

user1=foo

config_bash() {
    rm ~/.bash_profile
    ln -s ~/root/config/bash/login.sh ~/.bash_profile
    source ~/.bash_profile
    echo "Bash configured."
}
config_git() {
    local a="$1"
    if [ $# -eq 0 ]
    then
        a="default"
    fi
    if [ $# -eq 1 ]
    then
        git config --global user.name "$a"
        git config --global user.email "$a@$a.com"
    fi
    git config --global push.default matching
    git config --global core.editor "subl"

    git remote add godaddy $user1@foo.com:~/root.git
    git remote add heroku https://git.heroku.com/frozen-dusk-2587.git
    echo "Git configured."
}
config_grunt() {
    sudo npm install -g grunt-cli
    mkdir ~/root_install/grunt
    ln -s ~/root/config/grunt/package.json ~/root_install/grunt/package.json
    ln -s ~/root/config/grunt/Gruntfile.js ~/root_install/grunt/Gruntfile.js
    cd ~/root_install/grunt
    npm install
    cd ~/root
    echo "Grunt configured."
}
config_sublime_2() {
    ln -s /Applications/Sublime\ Text\ 2.app/Contents/SharedSupport/bin/subl /usr/local/bin/subl
    rm  -rf ~/Library/Application\ Support/Sublime\ Text\ 2/Packages/User
    ln -s ~/root/config/sublime ~/Library/Application\ Support/Sublime\ Text\ 2/Packages/User
    echo "Sublime configured."
}
reset_sublime_2() {
    rm -rf ~/Library/Application\ Support/Sublime\ Text\ 2
}
config_chrome(){
    chmod 0444 ~/Library/Application\ Support/Google/Chrome/Default/History
    echo "Chrome configured."
}
config_all() {
    config_bash
    config_git
    config_grunt
    config_sublime_2
    config_chrome
    list
}
list(){
    local a=$(which bash) b=$(which git) c=$(which grunt) d=$(which subl)
    local g=$(which node)  i=$(which heroku)
    echo "****"
    echo "Your bash executble is here: $a."
    echo "Your git executble is here: $b."
    echo "Your grunt executble is here: $c."
    echo "Your sublime executble is here: $d."
    echo "Your node executble is here: $g."
    echo "Your heroku executble is here: $i."
    echo "****"
}

Simplify

Since $# cannot be both 0 and 1 at the same time, it would be better to join the two if conditions here:

local a="$1"
if [ $# -eq 0 ]
then
    a="default"
fi
if [ $# -eq 1 ]
then
    git config --global user.name "$a"
    git config --global user.email "$a@$a.com"
fi

But, actually, what if there are 2 or more parameters instead of 0 and 1? It can happen by a programming mistake. It would be more robust to rewrite this by checking the values of the parameters, rather than their total count:

if [ "$1" ]
then
    git config --global user.name "$1"
    git config --global user.email "$1@$1.com"
fi

Notice that I eliminated the a local variable, as it was pointless (a="default" set but never actually used).

Avoid changing directories in scripts

This is a bit fragile:

cd ~/root_install/grunt
npm install
cd ~/root

A cd command may fail, for example if the directory was not yet created. In general it's not a good idea to change directories inside a script, as the script might perform actions in unexpected directories when things go wrong.

A good solution in such situations is to wrap commands within (...), like this:

(cd ~/root_install/grunt; npm install)

Directory changes executed inside (...) will not affect the environment outside.

You can learn more about this syntax in the Compound Commands section in man bash:

   (list) list  is  executed in a subshell environment (see COMMAND EXECU-
          TION ENVIRONMENT below).  Variable assignments and builtin  com-
          mands  that  affect  the  shell's  environment  do not remain in
          effect after the command completes.  The return  status  is  the
          exit status of list.

Robustness

Note that if you re-run the script a 2nd time, some commands will fail, for example these:

mkdir ~/root_install/grunt
ln -s ~/root/config/grunt/package.json ~/root_install/grunt/package.json
ln -s ~/root/config/grunt/Gruntfile.js ~/root_install/grunt/Gruntfile.js

If the directory and the symlinks already exist, the script will print error messages for these lines. You can avoid that by adding some flags:

mkdir -p ~/root_install/grunt
ln -sf ~/root/config/grunt/package.json ~/root_install/grunt/package.json
ln -sf ~/root/config/grunt/Gruntfile.js ~/root_install/grunt/Gruntfile.js

Naming

These variable names are not very helpful:

local a=$(which bash) b=$(which git) c=$(which grunt) d=$(which subl)
local g=$(which node)  i=$(which heroku)

Better name these after the commands they represent:

local bash=$(which bash) git=$(which git)  # and so on ...


Track mouse coordinates Problem Statement
  Write a JavaScript program that prints the mouse coordinates in a text area when the mouse cursor is moved over the HTML document.

  Expected Output

Solution

<!DOCTYPE html>

        <meta charset="UTF-8">
        <title>Track mouse coordinates</title>

Above code is written using deprecated API captureEvents.MDN says, captureEvents is deprecated.

Code still works fine with this deprecated API.

Can this code be improved, by replacing captureEvents with a better standard API?

mie isn't a good variable name. usesIE would be better, or well.. something like that!

Why is mouseX and mouseY defined outside your method's scope? They're useless there.

I don't think keeping variable at the root of the script is a good practice. Why don't you wrap it inside a window.onload call?

The only comment in your code seems useless. You do not use document.write anywhere in your code.

Also, when you have the choice, your conditions should be positive. It's just a little easier to read.

I don't see the use of returning true in a case like that. It means nothing.

Finally, I think that it'd be better to pass the usesIE parameter to the mousePos function. By the way, mousePos should be renamed writeMousePosition.

This is what the final code would look like :

function writeMousePosition(args,usesIE) {
    var mouseX = 0;
    var mouseY = 0;

    if(usesIE) {
        mouseX = args.clientX + document.body.scrollLeft;
        mouseY = args.clientY + document.body.scrollRight;
    } else {
        mouseX = args.pageX;
        mouseY = args.pageY;
    }

    document.body.innerHTML += "X: " + mouseX + ";Y: " + mouseY + ";  Time: " + new Date() + "<br>";
}

window.onload = function() {
    var usesIE = (navigator.appName == "Microsoft Internet Explorer") ? true : false;

    if(!usesIE) {
        document.captureEvents(Event.MOUSEMOVE);
    }

    document.onmousemove = function(args) {
        writeMousePosition(args, usesIE);
    };
};


Sorting a list by the modulus of 3 The input is an unsorted array of integers. The output should be an array of integers sorted based on the modulus of 3 (or any other number).

It begins with all integers conforming to Integer % 3 == 0 then 1 and then 2.

public class sortByMod {
public static void main(String args[]) throws Exception {

    int a[] = {1,90,2100, 12, 24, 80, 90, 5};
    LinkedList<List<Integer>> a1=sortMod(a);
    for(int i=0; i <a1.size();i++)
            System.out.print(a1.get(i)+" ");
}

public static LinkedList sortMod(int a[]){
    HashMap<Integer, TreeSet<Integer>> map = new HashMap<Integer, TreeSet<Integer>>();
    for(Integer i =0; i<a.length;i++){
        Integer modKey = a[i]%3;
        if(!map.containsKey(modKey)){
            map.put(modKey, new TreeSet<Integer>());
        }
        TreeSet<Integer> temp = map.get(modKey);
        temp.add(a[i]);
        map.put(modKey,temp);
    }
    LinkedList<List<Integer>> result= new LinkedList<List<Integer>>();
    TreeSet<Integer> temp;
    for(int i=0;i<=2;i++) {

        if(map.containsKey(i)){
           temp = map.get(i);
        }
        else return null;

        List<Integer> list = new ArrayList<Integer>(temp);
        result.add(list);

    }
    return result;
}
}

Your code definitely doesn't work with any number except 3, and it's quite likely you'll make an error if you'll be changing the code to another number. So I'd use

public LinkedList sortMod(int a[], int number)

instead. Also, the code returns null when there's no numbers of any modulus. I.e. sortMod(new int[]{1}) returns null as well as sortMod(new int[]{1, 4, 7, 10, 13}).

You can also try to use Streams, feature of Java 8. For example, the single line

return map.values().stream().map(ArrayList::new).collect(Collectors.toList());

does a lot of job, effectively replacing 10 lines of your code. You can try to make the task in two lines using Java 8 features. Also it works fine with sortMod(new int[]{1, 4, 7, 10, 13}).


Reusable user control (ascx) design for multiple web application My company's web application projects are heavily based on user controls. One common case  is like we want to reuse the UI visual elements but note the packaged original logic .

I have read some very smelly code in some user controls code behinde.

if(WebConfig.CurrentProject == "xxx")
{
    //do xxx's logic
    ddlPostalCode.DataSouce= this.addressManager.getUSAPostal();
}
else
{
    //do yyy's logic
    ddlPostalCode.DataSouce= this.addressManager.getUKPostal();
}

//Or use configuration
if(config.enableAlpha)
{
   //Do alpha's logic
    ddlPostalCode.DataSouce= this.addressManager.getAlphaPostalCode();
}

And these logics normally involving manipulating server controls .

So I am thinking, if we just want to reuse the visual element and parts of its logic.
Why don't we make the server and its event handler public ?

//In xxx.ascx.cs
public EventHandler AddressControl_Init;

protected override void OnInit(EventArgs e)
{
     if(this.AddressControl_Init!= null)
     {
         this.AddressControl_Init(sender, e);
     }
     else
     {
        //Default logic.
     }
}

//In xxx.ascx.designer.cs
public  public global::XYZ.Controls.Basic.XYZDropDownList ddlPostalCode;

//In the code behinde of the page using this address ascx.
//HostingPage.aspx.cs
protected override void OnPreInit(EventArgs e)
{
    this.AddressControl.AddressControl_Init += new EventHandler(USAAddressControl_Init);
}

void USAAddressControl_Init(object sender, EventArgs e)
{
    //Now i can use any business manager class here to populate the source data.
    this.AddressControl.ddlPostalCode.DataSource = Anymanager.GetMarsCode();
    this.AddressControl.ddlPostalCode.DataBind();
}

This is a very simple example, but it also can be a very flexsiable solution.
It decouple the logic behinde the user control. Let the user, the page to deside the control's logic.

Is there any better elegent ways to solve my problem?

Is there any better elegent ways to solve my problem?

You are going down a horrible path messing with all these user controls.  ASP.NET MVC is much better for dealing with reusable visual parts, and everything happens on the request level so you don't have to worry about horrible event ordering.

Can't use MVC?  Forget the complex events of user controls entirely.  Put no code in user controls.  If you need the data from a textbox in a control for example, pull it directly from the post variables (wrap this in a class if you need to)

Make your goal to deal with requests in a SINGLE, centralized point.  It's the MVC way, it's the MVP way, it's the MVVM way.

Need ajax?  scrap ajax controls and hit a web service;  throw away the ajax toolkit.

I actually wish I had an old crufty webforms app that I could re-work this on.  I would completely bail on the failure that is asp.net lifecycle


Converting unsigned integer value into byte array I have a function that takes an unsigned value and transforms it into a std::vector<std::uint8_t>. The requirements (and post conditions) are as follows:

The code must utilize "modern C++" up to C++14
The focus is on readability/maintainability first, performance second
The most significant byte must be at index 0.
The lease significant byte must be at index size()-1.
The return type cannot be changed; this is a requirement stemming from other code not shown in this context (larger application requirement). Also avoids a conversion from one container to std::vector.
The resulting vector must be as compact as possible. For values that decompose into fewer bytes than sizeof(unsigned), the resulting vector must not contain unnecessary MSB's (that is, zero value bytes).
The code must be endianness-agnostic and portable to various other architectures and platforms.

Here is the current implementation I have:

using BinaryStreamByteVector = std::vector<std::uint8_t>;

BinaryStreamByteVector CreateMultiByteInteger(unsigned value)
{
    BinaryStreamByteVector value_bytes;

    while (value != 0)
    {
        std::uint8_t byte = value & 0xFF;
        value_bytes.insert(value_bytes.begin(), byte);

        value >>= 8;
    }

    return value_bytes;
}

(Live Sample on Coliru)

I can't think of a better implementation for CreateMultiByteInteger, so I'm hoping for other ideas that meet the requirements above.

Observations

Your code is beautiful (proper naming conventions followed, proper whitespace usage, very readable. I particularly like the type-aliasing, which helps make the whole thing that much more readable than a typedef or #define). I cannot personally think of any more ways to make your code more readable, however, I do have 1 suggestion.

The algorithm you currently follow is standard and ideal (you even have proper masking of higher-order bits); and in my honest opinion any other algorithm would be tantamount to obfuscation; but if you assume that the code will only ever run on a particular architecture, you could probably unroll the loop.

Suggestion

Make your function parameter const. Although C++ is call-by-value, making it explicitly const and mutating a local copy of the variable is never a bad idea, and it does make your function contract that much clearer.
On the performance side, since on any reasonable computer unsigned will have maximally 16 bytes (64 bits of 8 bytes is the case for all common architectures, except for probably some microcontrollers which will have less, even as less as 1 byte) (note that the upcoming RISCV architecture does have a native 16-byte (128-bit) integral data type, but the 64-bit one is the default), you can easily make this function a template, such that the computation of the byte vector can be done at compile time without much overhead and incur no runtime overhead.

You can probably even pre-set the capacity of your std::vector utilising sizeof(unsigned).

Although I'm not sure if this suggestion will work in the constraints of your application.


Intersect channels in Go I have a simple golang app in which I have two background tasks that produce data,each of them use it's own channel to signal it produced another unit of data.
I have a single consumer that needs the data form the two other background tasks as input in a 1-to-1 ratio. What I want to do,for the lack of a better term, is "Intersecting" the channels. Meaning I want to create a new channel that gets a signal every time there's a signal in one channel and then the other.
it's also worth mentioning I have a context I use to close all background gorotines to gracefully shutdown.
here is the function I wrote to do it:
func intersectChans(a <-chan bool, b <-chan bool) {
    for {
        select {
        case <-a:
            select {
            case <-b:
                c <- true
            case <-ctx.Done():
                return
            }
        case <-ctx.Done():
            return
        }
    }
}

I wonder if there is a more elegant/idiomatic way to achieve this

"Actually you code is pretty much this. Simple and boring as most other good Go code" I thought.
At this point I could say "Wrap your c <- true with a select statement, add the third case <-ctx.Done: to handle possible deadlock and you are ready to go".
But.
Does it really should be this way? Yep, this works and does the job, but are there any pitfalls we missed? Let see.
Indentation
Let's talk about idiomatic Go. Within the space of idiomatic Go a proper indentation is a single Tab rendered as 8 spaces:
func intersectChans(a <-chan bool, b <-chan bool) {
        for {
                select {
                case <-a:
                        select {
                        case <-b:
                                select {
                                case c <- true:
                                case <-ctx.Done():
                                        return
                                }
                        case <-ctx.Done():
                                return
                        }
                case <-ctx.Done():
                        return
                }
        }
}

Okay, the first pitfall we have is a deep code indentation.
Well, not really a problem because, khm, Idiomatic Go, it is you, who decided to render 8 spaces for a single Tab in the first place. And the Linux kernel C style guide as well.
Actually there is a good reason for this. Let me quote the style guide I linked above:

Now, some people will claim that having 8-character indentations makes the code move too far to the right, and makes it hard to read on a 80-character terminal screen. The answer to that is that if you need more than 3 levels of indentation, you’re screwed anyway, and should fix your program.

OK, we definitely have "too far to the right" movement here. Let's consider this a pitfall, but not a critical one.
Channels
This code is all about the channels. We have 4 here:

a and b channels to receive data
c channel to send aggregated results from a and b
ctx.Done() channel to terminate the goroutine

What can we do with channels? We can:

receive
send
and close the channel

ctx.Done() channel is closed to tell goroutine that the task is done. But what about our own channels?

You needn't close every channel when you've finished with it. It's only necessary to close a channel when it is important to tell the receiving goroutines that all data have been sent. A channel that the garbage collector determines to be unreachable will have its resources reclaimed whether or not it is closed.

says some book about Go.
OK, so we can close a channel to notify its reader, that all data has been sent.
But a read from a closed channel still returns some data. It will be a zero value of the channel data type. To actually check, that the channel was closed, we can use the "comma ok" idiom:
data, ok := <- a
if !ok {
        fmt.Println("a was closed")
}

OK, but why will we do it in the first place?
A not so boring solution
If we combine our strive to have a more flat code with an ability to check for closed channels we can come up with a solution like this:
func join(a, b <-chan int, r chan<- []int) {
        defer close(r)
        for {
                v1, ok1 := <-a
                v2, ok2 := <-b
                if !(ok1 && ok2) {
                        return
                }

                r <- []int{v1, v2}
        }
}

In this code sample we are using "comma ok" idiom to test for a closed worker channels and stop processing.
This code relies on the fact, that worker goroutines will close their channels to notify us, that all data has been sent. We do the same and close our channel with a defer statement.
And here are the missing parts (also on Go Playground):
package main

import (
        "context"
        "fmt"
        "time"
)

func worker(ctx context.Context, c chan<- int) {
        defer close(c)
        for i := 0; ; i++ {
                select {
                case <-ctx.Done():
                        return
                case c <- i:
                }
                time.Sleep(time.Second / 4)
        }
}

func join(a, b <-chan int, r chan<- []int) {
        defer close(r)
        for {
                v1, ok1 := <-a
                v2, ok2 := <-b
                if !(ok1 && ok2) {
                        return
                }

                r <- []int{v1, v2}
        }
}

func main() {
        ctx, cancel := context.WithTimeout(context.Background(), time.Second)
        defer cancel()

        a := make(chan int)
        b := make(chan int)
        r := make(chan []int)

        go worker(ctx, a)
        go worker(ctx, b)
        go join(a, b, r)

        for pair := range r {
                fmt.Printf("got %+v\n", pair)
        }
}

"So, which solution should I take?" will you ask.
It depends. One solution is a simple and boring Go code. The other is "not that far to the right", but has some fancy channel operations not everybody even know exists.
And while you decide, don't forget to fix a possible deadlock in your actual code :)


Python Command Line Rock Paper Scissors I'm looking for tips on how to clean up this code and/or make it more efficient.

Keep in mind I am a seriously new programmer and if it is too advanced it's likely to go over my head.

import random
cscore = 0
pscore = 0
print 'I would like to play a game. \nThe name of the game is Rock, Paper, Scissors.'
rest = 'y'
while (rest == 'y'):
    pick = ['r', 'p', 's']
    b = (random.choice(pick))
    a = raw_input ('\nFirst Letter ')
    if a == 'r' or a == 'R':
        a = 'Rock'
    elif a == 'p' or a == 'P':
        a = 'Paper'
    elif a == 's' or a == 'S':
        a = 'Scissors'

### assign name to computer's answer
    if b == 'r':
        b = 'Rock'
    elif b == 'p':
        b = 'Paper'
    elif b == 's':
        b = 'Scissors'
    print 'You picked ' + a + '\nAnd I picked ' + b

##### compare picks
    if a == 'Rock' and b  == 'Rock':
        print 'Silly, we both picked Rock, \nthat means we tied.'
    elif a == 'Rock' and b == 'Paper':
        print 'Yes! Paper smothers Rock, therefore, I win.'
        cscore = cscore + 1
    elif a == 'Rock' and b == 'Scissors':
        print 'Crap, Rock smashes Scissors. You win.'
        pscore = pscore + 1
    elif a == 'Paper' and b  == 'Paper':
        print 'Silly, we both picked Paper, \nthat means we tied.'
    elif a == 'Paper' and b == 'Scissors':
        print 'Yes! Scissors cut Paper, therefore, I win.'
        cscore = cscore + 1
    elif a == 'Paper' and b == 'Rock':
        print 'Crap, Paper smothers Rock. You win.'
        pscore = pscore + 1
    elif a == 'Scissors' and b  == 'Scissors':
        print 'Silly, we both picked Scissors, \nthat means we tied.'
    elif a == 'Scissors' and b == 'Rock':
        print 'Yes! Rock smashes Scissors, therefore, I win.'
        cscore = cscore + 1
    elif a == 'Scissors' and b == 'Paper':
        print 'Crap, Scissors cut Paper. You win.'
        pscore = pscore + 1
    print '\nThe score is now Computer ' + str(cscore) + ' Human ' +     str(pscore)
    rest = raw_input ('\nWould you like to play again? y or n ')
print '\nThank you for playing. The final score was Computer ' + str(cscore) + ' Human ' + str(pscore)
if cscore > pscore:
    print 'Ha!, I won and you lost.'
elif cscore == pscore:
    print 'We tied in the overall score, you are a worthy opponent.'
elif cscore < pscore:
    print 'You beat me, you low down mangy son of a gun. You must have cheated.'

You used a massive number of if and elif statments, I suggest using Math
You used no functions, I suggest using some.
You are repeating yourself very much, I suggest to say things once and only once.
You are mixing logic and user interface, I suggest not to.
You are not automatically testing your code, changing it may break it.

My implementation does not count wins and losses and is a bit hard to follow but was developed following the above principles that I believe make good code:

import doctest
import random

ROCK = 0
PAPER = 1
SCISSORS = 2
SIGNS = ['ROCK', 'PAPER', 'SCISSORS']

LETTER_TO_RPS = {
    'r':ROCK,
    'p':PAPER,
    's':SCISSORS
    }

def rps_compare(a, b):
    """
    >>> rps_compare(ROCK, SCISSORS)
    'WIN'
    >>> rps_compare(PAPER, SCISSORS)
    'LOSS'
    """
    if a == b: return 'TIE'
    return 'WIN' if (a - b) % 3 == 1 else 'LOSS'

def rps_round(letters_map=LETTER_TO_RPS):
    user_choice = LETTER_TO_RPS[raw_input("[R]ock, [P]aper or [S]cissors? ").lower()]
    cpu_choice = random.choice([ROCK,PAPER,SCISSORS])
    print("I chose {}.".format(SIGNS[cpu_choice]))
    return rps_compare(user_choice, cpu_choice)

def rps_game(welcome_message, letters_map=LETTER_TO_RPS):
    print(welcome_message)
    while True:
        print("This is a {}".format(rps_round()))

if __name__ == "__main__":
    doctest.testmod()
    rps_game('Hello')


Re-using this piece of code for different objects To give a little context before showing the problem:

I am creating a RESTful web application. A User can have multiple addresses, emails, phones and projects. Each of these is represented similar to the following object model, (Address in this case).

public class Address {
    String type; //home or work etc.
    String streetname;
    /*some more things*/

    Privacy privacy; //Privacy object
}

Every model contains a privacy object since a user can decide which of his addresses may be set to private (for instance only show the address to his contacts). The privacy object contains the following:

public class Privacy {
    @Id
    private String id;
    private boolean showForNonContacts;
    private boolean onlyShowForGroups;

    /*
    getters and setters
    */
}

The problem:

In my controller, I am fetching the userprofile object which contains these models. For every model, I have to check the following:

List<Address> visibleAddresses = new ArrayList<Address>();
for(Address address : userProfile.getAddresses()) {
    if(address.getPrivacy().isShowForNonContacts() || hasContact) {
        visibleAddresses.add(address);
    }
    userProfile.setAddresses(visibleAddresses);
}

//Same code repeats but now for email/phone/projects

I have to do this for the emails, phones and projects. This clearly is a bad design and I would like to make it better and cleaner than repeating the above code for every model.
Does anybody have any tip of how I can make the above code work? I thought of using generics, interfaces where this method would be some what common, but I really don't know since I'd like to keep the model class clean.

Any tips/advice is greatly appreciated.

The UserProfile class should still contain a List for instance, since my db mapper (MongoDB/Spring data) has to recognize it as an embedded object.

EDIT:
Based on the answer of Greg Burghardt, I decided to create a generic class called GenericCollection. Each of the models implements a separate interface.
GenericCollection extends this interface and makes use of the method which controls the privacy (as defined in the interface).

Now the code is a lot cleaner.

How about creating an AddressCollection class that holds all the logic of determining the "visible" addresses:

public class AddressCollection
{
    private ArrayList<Address> items;

    public AddressCollection()
    {
        items = new ArrayList<Address>();
    }

    public AddressCollection(ArrayList<Address> addresses) {
        items = addresses;
    }

    public void add(Address address) {
        items.add(address);
    }

    public ArrayList<Address> getVisibleAddresses() {
        ArrayList<Address> visibleAddresses = new ArrayList<Address>();

        for (Address addr : items) {
            if (addr.getPrivacy().isShowForNonContacts()) {
                visibleAddresses.add(addr);
            }
        }

        return visibleAddresses;
    }

    public ArrayList<Address> getItems() {
        return items;
    }
}

Each model that needs this functionality is reduced to a little boiler plate code, but at least you aren't repeating the basic domain logic:

public class UserProfile
{
    private AddressCollection addresses;

    public UserProfile() {
        addresses = new AddressCollection();
    }

    public void setAddresses(ArrayList<Address> addresses) {
        this.addresses = new AddressCollection(addresses);
    }

    public ArrayList<Address> getAddresses() {
        return addresses.getItems();
    }

    public ArrayList<Address> getVisibleAddresses() {
        return addresses.getVisibleAddresses();
    }
}

public class SomethingElse
{
    private AddressCollection addresses;

    public UserProfile() {
        addresses = new AddressCollection();
    }

    public void setAddresses(ArrayList<Address> addresses) {
        this.addresses = new AddressCollection(addresses);
    }

    public ArrayList<Address> getAddresses() {
        return addresses.getItems();
    }

    public ArrayList<Address> getVisibleAddresses() {
        return addresses.getVisibleAddresses();
    }
}


Check whether address criteria contain only certain fields or more I have a method that checks whether an addressCriteria contains only certain fields or more.  If it contains more fields, it should return false, otherwise true.

I'm uncertain what the best way to write my if statement is. As far as I can tell, I see 2 options:

Write them in 1 if statement
Write each check in a different if statement

Single if statement:

private boolean isShortAddress(AddressCriteria addressCriteria) {
  if((null != addressCriteria.getCity() && !addressCriteria.getCity().isEmpty()) ||
      (null != addressCriteria.getNumber() && !addressCriteria.getNumber().isEmpty()) ||
      (null != addressCriteria.getState() && !addressCriteria.getState().isEmpty()) ||
      (null != addressCriteria.getZipCode() && !addressCriteria.getZipCode().isEmpty())) {
    return false;
  }

  return true;
}

Multiple if statements:

private boolean isShortAddress(AddressCriteria addressCriteria) {
  if(null != addressCriteria.getCity() && !addressCriteria.getCity().isEmpty()) {
    return false;
  }

  if(null != addressCriteria.getNumber() && !addressCriteria.getNumber().isEmpty()) {
    return false;
  }

  if(null != addressCriteria.getState() && !addressCriteria.getState().isEmpty()) {
    return false;
  }

  if(null != addressCriteria.getZipCode() && !addressCriteria.getZipCode().isEmpty()) {
    return false;
  }

  return true;
}

Maybe there's a 3rd or 4th option that I am unaware of?

I would be compelled to write a helper that accepts a variable number of address fields and and returns true if all of the address fields are not null and not empty.

private boolean isShortAddress(AddressCriteria addressCriteria) {

  return hasFields( addressCriteria.getCity(),
                    addressCriteria.getNumber(),
                    addressCriteria.getState(),
                    addressCriteria.getZipCode() );
}

private boolean hasFields( addressField... args )
{
  for( addressField arg:args)
  {
    if( null == arg || arg.isEmpty() )
      return false;
  }
  return true;
}

I did not test this code, but you should catch the drift.


Unit testing a click event with asynchronous content I'm writing unit tests for a web application that has quite a lot of UI interactions. I would like some feedback regarding on how to handle click events with asynchronous code.

My goal here is to simluate users interacting with the page. That's why I'm triggering events rather than making function calls.

Here is my idea so far:

$('#foo').on('click', function(){

     var dfd = $.Deferred();

     simulateAsync().done(dfd.resolve).fail(dfd.reject);
     $(this).data({ promise: dfd.promise() });
});

function simulateAsync(){

    var dfd = $.Deferred();

    setTimout(function(){
        $('body').text('foo');
        dfd.resolve();
    }, 1000);

    return dfd.promise();
}

Testing the code:

test('foo text set on body after 1000ms when clicking #foo', function(assert){

    var $foo = $('#qunit-fixture').find('#foo'),
        done = assert.async();

    $foo.trigger('click');

    var completePromise = $foo.data('promise');

    completePromise.done(function(){
        assert.equal($('#bar').text(), 'bar');
        done();
    });
});

http://jsfiddle.net/0m4t4ndt/2/

Without the real code to be tested, I can only provide an example of how I've tested something similar with QUnit and Sinon.JS, though you don't even need the latter.

You can either use Sinon.JS to mock a response to the AJAX call or substitute your own manual mock for $.getJSON or $.ajax. I find the former preferable as it documents the AJAX call and tests the full front-end code, but it's beyond the scope of this question. You could argue that you're needlessly testing jQuery itself, and you'd be right. YMMV :)

Mocking JQuery

Let's assume this click event handler that uses the /load-foo AJAX call to acquire a JSON string. I've omitted the extra deferred for simplicity as your current test of the promise attached to the #foo element is fine.

$('#foo').on('click', function(){
    $.getJSON('/load-foo', {
        foo: 'bar'
    }).done(function (foo) {
        $('body').text(foo);
    });
});

Here's how you would test this handler.

test('foo text via AJAX set on body after clicking #foo', function(assert) {
    var $foo = $('#qunit-fixture').find('#foo'),
        getJSON = $.getJSON;

    $.getJSON = function (url, data) {
        assert.equal(url, '/load-foo');
        assert.equal(data.foo, 'bar');
        return new $.Deferred().resolve('"foobar"');
    }

    $foo.trigger('click');

    assert.equal($('body').text(), 'foobar');
    $.getJSON = getJSON;
});

For safety, I'd recommend saving and restoring $.getJSON in the module's setup and teardown functions. If you don't, any exception thrown in the test will leave the mock in place.

Mocking AJAX

While this does result in a lot of boilerplate sometimes, you can use Sinon's fake server to mock the AJAX responses. The beauty here is that it will match up the request to the response to ensure your AJAX calls (URL, data, type, etc.) are all correct and then supplies a canned response.

Here's the same test above using a fake server.

module('AJAX tests', {
    setup: function () {
        this.server = sinon.fakeServer.create();
    },
    teardown: function () {
        this.server.restore();
    }
});

test('foo text via AJAX set on body after clicking #foo', function(assert) {
    var $foo = $('#qunit-fixture').find('#foo');

    // setup the expected request with the canned response
    this.server.respondWith('/foo',
        [ 200, { 'Content-Type': 'text/json' }, '"foobar"' ]
    );

    $foo.trigger('click');

    // handle queued requests immediately
    this.server.respond();

    assert.equal($('body').text(), 'foobar');
});

If you're mocking a lot of JSON AJAX requests, you can create a common fixture to make this a little saner. You can call this.mockAjax with a number for an HTTP response code, an object to convert it to JSON, or a string as the raw response.

var ajaxFixture = {
    setup: function () {
        this.server = sinon.fakeServer.create();
    },
    teardown: function () {
        this.server.restore();
    },
    mockAjax: function (url, response) {
        var type = { 'Content-Type': 'text/json' };
        if (typeof response === 'number') {
            this.server.respondWith(url, [ response, type, '' ]);
        }
        else if (typeof response === 'object') {
            this.server.respondWith(url, [ 200, type, JSON.stringify(response) ]);
        }
        else {
            this.server.respondWith(url, [ 200, type, response ]);
        }
    }
);

Then you simply use this as the module's fixture:

module('AJAX tests', ajaxFixture);

test(...);

Non-Mocking Solution

As you say in your comment, you want to execute the full AJAX call to your backend service.

  Note: This is not unit testing anymore but rather functional or integration testing. Unit testing involves testing each small piece (unit) in isolation of the others: the click handler. Adding in jQuery, XMLHTTPRequest, and your backend service will make isolating bugs more difficult. But it is useful for making sure all the pieces work together.

  I still recommend doing actual unit tests in addition to integration testing for quicker development as they typically run much faster and can be executed with every change to a source/test file from your IDE.

First, modifying your click handler invalidates any test you run, so we'll need to leave that in place, i.e., no simulated call or extra deferred. Your best bet is to poll for the call to complete with an interval and fail the test if it doesn't complete within X seconds.

test('foo text via AJAX set on body after clicking #foo', function(assert) {
    var $foo = $('#qunit-fixture').find('#foo'),
        $body = $('body'),
        done = assert.async(),
        timeout = Date.now() + 5000, // timeout after 5s
        timer;

    $foo.trigger('click');

    timer = window.setInterval(function () {
        if ($body.text() === 'foobar') {
            assert.equal($body.text(), 'foobar');
            window.clearInterval(timer);
            done();
        }
        else if (Date.now() >= timeout) {
            assert.ok(false, 'AJAX timed out');
            window.clearInterval(timer);
            done();
        }
    }, 250);
});

Obviously, this won't work if you're using Sinon's fake timers, so disable them for this test if enabled globally. Also, you'll need to figure out if a slow AJAX call will interfere with later tests when it finally completes and puts "foobar" into the body. Maybe you want to put it into the fixture itself instead of polluting the global body.


Implementing SQL function STR I am writing a piece of code in c# that mimics the behaviour of following SQL function STR in the following case:

declare @number numeric(19,15)    = 219.37345462345234235
SELECT str(@number,9,9) -- 219.37345

declare @number numeric(19,15)    = 19.37345462345234235
SELECT str(@number,9,9) -- 19.373455

Following is an input output table for this scenario:

I have implemented the following function in C# which accepts a double variable and returns a string which is equal to the result returned by the SQL function STR(@number,9,9).

public static string GetCustomStringQuantity(double quantity)
    {
        string customQuantity ;
        int length = (int) Math.Log10(Math.Abs(quantity))+1;
        if (length>9)
            return "*********";
        quantity = Math.Round(quantity, 8 - length,MidpointRounding.AwayFromZero);
        customQuantity = quantity.ToString();
        customQuantity= (quantity %1==0)?(customQuantity+".").PadRight(9, '0'):customQuantity.PadRight(9, '0');

        if (customQuantity.Length > 9)
            customQuantity = customQuantity.Substring(0, 9);
        else if (customQuantity.Length == 9 && customQuantity.EndsWith("."))
            customQuantity = customQuantity.Substring(0, 8);

        return customQuantity;
    }

How can I make this function better? Or is there a better way to implement this functionality?

Once you have the length of the integer portion, you can use the fx format specifier to get the correct number of decimals, which would clean up the string building part.

 return quantity.ToString("f" + (8 - length));

That should clean up the code some.

As I started typing that, I figured I'd check for edge cases too - since STR is filled with them (eg., it includes the negative sign in the length, returns ** if it overflows, etc.). You're handling the ** case, but a couple of edge-case values fail:

 Input       | Expected  | Actual
 999999999.5 | ********* | 1000000000
 123456789   | 123456789 | ArgOutOfRangeException
 123456789.5 | 123456790 | ArgOutOfRangeException
 -123456789  | ********* | ArgOutOfRangeException

I'd advise that if you're looking for 100% compatibility with STR here, you'll probably want some extensive unit tests.

Here's what I came up with that handles the edge-cases I threw at it, but I don't expect it's bullet proof either. Main changes are using the format specifier, and using .ToString to get the number of digits before the decimal place instead of Log10.

 public static string GetCustomStringQuantity(double quantity) {
    const int LENGTH = 9;
    // Include negative sign, just like STR()
    int numberOfIntDigits = Math.Round(quantity, MidpointRounding.AwayFromZero).ToString().Length;
    if (numberOfIntDigits == LENGTH) return quantity.ToString("f0");
    if (numberOfIntDigits > LENGTH) return new string('*', LENGTH);

    int numberOfDecimals = LENGTH - numberOfIntDigits - 1; // include 1 for the decimal place
    return quantity.ToString("f" + numberOfDecimals); // uses MidpointRounding.AwayFromZero
}


Checking inactivity with periodic tasks in Java I have created a simple HealthCheck service. I have two approaches:

Create a task canceling the previous one if the system is active, rescheduling the new task.
Create a periodic task and regularly check the last active time.

Which one is better?
Recommendations on how to improve my usage of synchronized, volatile, AtomicBoolean keywords in both approaches above are welcome.
public interface IHealthCheck {
    void start();
    void stop();
    void onActive();
    void onExpire();
}

public class Main {
    public static void main(String[] args) {
        IHealthCheck check = new HealthCheck2();

        new Thread(() -> checkTask(check, 5)).start();

        new Thread(() -> checkTask(check, 6)).start();

        new Thread(() -> checkTask(check, 7)).start();

        sleep(100);
        check.start();

    }

    private static void checkTask(IHealthCheck check, long sleep) {
        for (int i = 0; i < 1000; i++) {
            sleep(sleep);
            check.onActive();
        }
    }

    private static void sleep(long n) {
        try {
            Thread.sleep(n);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
}

package com.deneme;

import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

public class HealthCheck implements IHealthCheck {

    private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor();
    private ScheduledFuture<?> future;

    @Override
    public void start() {
        onActive();
    }

    @Override
    public synchronized void stop() {
        if (future != null) {
            future.cancel(true);
        }
    }

    @Override
    public synchronized void onActive() {
        if (future != null) {
            future.cancel(true);
        }
        System.out.println("Active: " + System.currentTimeMillis());
        future = scheduler.schedule(this::onExpire, 200, TimeUnit.MILLISECONDS);
    }

    @Override
    public void onExpire() {
        System.out.println("Expired: " + System.currentTimeMillis());
    }
}

package com.deneme;

import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

public class HealthCheck2 implements IHealthCheck {

    private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor();
    private ScheduledFuture<?> future;
    private Instant lastActiveTime;
    private boolean expired;

    @Override
    public void start() {
        onActive();
        future = scheduler.scheduleWithFixedDelay(this::check, 50, 50, TimeUnit.MILLISECONDS);
    }

    private synchronized void check() {
        long diff = ChronoUnit.MILLIS.between(lastActiveTime, Instant.now());
        if (diff > 100) {
            onExpire();
        }
    }

    @Override
    public synchronized void stop() {
        if (future != null) {
            future.cancel(true);
        }
    }

    @Override
    public synchronized void onActive() {
        expired = false;
        lastActiveTime = Instant.now();
        System.out.println("Active: " + lastActiveTime.toEpochMilli());
    }

    @Override
    public void onExpire() {
        if (!expired) {
            expired = true;
            System.out.println("Expired: " + System.currentTimeMillis());
        }
    }

}

It depends on your needs, but generally I would say that the second approach would be better.
Why? because it allows you to run multiple tasks and that gives flexibility to your system. Now the use of synchronized may be troublesome if there are many threads running, if you don't need to handle static or shared resources/variables it would be better to not use it.
Now, in the first place your classes has no static variables, and does not access to shared resources so synchronization is not needed. In the case of the class HealthCheck2, the following method will work fine without synchronized
    private void check() {
        //the variable diff isn't needed either
        if (ChronoUnit.MILLIS.between(lastActiveTime, Instant.now()) > 100)
            onExpire();
    }

Why? Because the check method is uniquely owned by each HealthCheck2 instance, even if they call the start method, the scheduler and the future variables are equally owned.
I think you could remember when you were learning about static variables and methods that these are accessible and modifiable by any class instance. Well, the oposite is true, non-static variables and methods are unique for each instance.
Hope it helped you.


Calculating chmod based on user input This program asks the user for input three times, and based on that input, calculates the chmod. I have included comments with questions and clarifications in the code.

I would like to have these questions answered and help with making the if ... in ... part shorter, if that's possible.

'''
This program asks the user for input three times, and based on that input, calculates the chmod.
'''

'''
This is a simple function that prevents me from repeating myself.
Usage:
    whoCan('execute')
will return 'Who can execute?'
'''

def whoCan(doThis):
    '''
    If I use `print` instead of `return` here, this is the output:

     Who can read?
     None

    Why is that?
    '''
    return "Who can %s?" % doThis

'''
Ask the user for input in the form of 'ow', 'gr' and 'ot'.
FYI:
    ow = owner
    gr = group
    ot = other

chmod 777 would be  owgrot
                    owgrot
                    owgrot
'''

read = raw_input(whoCan('read'))
write = raw_input(whoCan('write'))
execute = raw_input(whoCan('execute'))

'''
I didn't have any success with
    readValue, writeValue, executeValue = int()
Why?
'''

readValue = 0
writeValue = 0
executeValue = 0

# There must be a shorter way to write this...

if 'ow' in read:
    readValue += 4
if 'gr' in read:
    readValue += 2
if 'ot' in read:
    readValue += 1

if 'ow' in write:
    writeValue += 4
if 'gr' in write:
    writeValue += 2
if 'ot' in write:
    writeValue += 1

if 'ow' in execute:
    executeValue += 4
if 'gr' in execute:
    executeValue += 2
if 'ot' in execute:
    executeValue += 1

# And print the chmod

print readValue, writeValue, executeValue

Concise coding

After moving the documentation of the function after the line containing def as the convention stands we have:

def whoCan(doThis):
    '''
    This is a simple function that prevents me from repeating myself.
    Usage:
        whoCan('execute')
    will return 'Who can execute?'

    If I use `print` instead of `return` here, this is the output:

     Who can read?
     None

    Why is that?
    '''
    return "Who can %s?" % doThis

The second paragraph is just a doubt that is not really relevant to this function (the reason is that: using print creates a function with no return of which the return is implicitly None, that is printed by raw_input), so we can remove it:

def whoCan(doThis):
    '''
    This is a simple function that prevents me from repeating myself.
    Usage:
        whoCan('execute')
    will return 'Who can execute?'
    '''
    return "Who can %s?" % doThis

I can already see that it is a (simple) function, and functions are usually used to avoid repetition so we can remove it too:

def whoCan(doThis):
    '''
    Usage:
        whoCan('execute')
    will return 'Who can execute?'
    '''
    return "Who can %s?" % doThis

In fact we may remove this whole function as it is just doing "Who can {}?".format(action) where x is the input.

To avoid repetition we will use a generator expression:

read, write, execute = (raw_input("Who can {}?".format(action)) for action in ('read', 'write', 'execute'))

The last part can be shortened with a dictionary and a function that calls sum on a generator expression:

name_to_val = {'ow' : 4, 'gr' : 2, 'ot' : 1}

def chmod_value(x):
    return sum(name_to_val(name) for name in name_to_val.keys() if name in x)

You also do not need to initialize variables with zero, in fact you do not need variables at all:

print [chmod_value(x) for x in (read, write, execute)]


Determining function return types for std::function I'm using the boost::units library (boost::units) and trying to make use of auto and decltype to deal with all the complicated types it produces.

I have a templated function to integrate a mathematical function (which might accept x values with units such as 1*metre) between two points:

template <class Out, class In >
auto integrate( std::function<Out(In)> const & f,
In xmin, In xmax, int nSteps)
-> decltype( f(xmin)  * xmin  )

Now I want to use this with some functions that have big ugly return types. I have written this:

//Standard quantities
quantity<temperature> stdBlackBodyTemp = 2857.0*kelvin;
auto luminousEfficiency = quantity<luminous_flux>(682.0*lumen) / quantity<power>(1.0 * watt);

//Integration range
quantity<length> minWavelength(150.0 * nano * metre);
quantity<length> maxWavelength(600.0 * nano * metre);

//Get type for spectral_differential_current
quantity<length> fakeLength(300.0 * nano * metre);

typedef decltype(pmt_radiant_sensitivity(fakeLength)
        * plancklaw(fakeLength, stdBlackBodyTemp)) sdctype;

//Define spectral_different_current function
std::function< sdctype(quantity<length>)>
    spectral_differential_current = [&stdBlackBodyTemp](quantity<length> l)
    {
        return
            pmt_radiant_sensitivity(l) * plancklaw(l, stdBlackBodyTemp );
    };

//Integrate over spectral_differential_current
auto differential_current = integrate( spectral_differential_current,
        minWavelength, maxWavelength, 100);

To define my std::function spectral_differential_current which I'm going to give to integrate I need to know the return type. The only way I have found to get this is by using the function on some random value (fakeLength) and then using decltype. It seems odd that I have to introduce a variable which isn't actually used just to get a type. Is there a better way of approaching this?

I'm not sure why you need the typedef in the first place. Is there anything wrong with auto spectral_differential_current = ...?

On a similar note, why do you want to require that f is an std::function, disallowing reasonable calls like integrate(sin, 0, 2, 10)? Most code of this type can happily accept anything that can be called:

template <typename F, typename In>
auto integrate(F f, In xmin, In xmax, int nSteps)
-> decltype(f(xmin)  * xmin)

The compiler will tell you whether a given f is OK. Usually by not finding a suitable integrate function – SFINAE.

Also note the use of typename – that's just a stylistic thing of course, but e.g., double is not a class, so I prefer not to say class for template parameters that would accept a double or a plain function pointer.


Draggable toolbar header class, inheriting from a draggable parent class I have written a draggable parent class using a module pattern:

var draggable = {};
(function(){
var initialMouseX = undefined;
var initialMouseY = undefined;
var startX = undefined;
var startY = undefined;
var draggedObject = undefined;
var startDragX = undefined;
var startDragY = undefined;
var eventHandler = undefined;
var canvas = undefined;
var that = this;

this.setCanvasObject = function(cnv){
 canvas = cnv;
};

this.setEventHandler = function(eH){
eventHandler = eH;
};
this.setClickable = function(element){ ... };

this.setDraggable = function(element){ ... };

this.startDragMode = function(ev){ ... };

this.dragMouse = function(ev){ .... };

this.setPosition = function(dX, dY){ ... };

this.releaseElement = function() {
...
};

}).apply(draggable);

Here is my child class, using the same module pattern:

var toolbarHeader = {};

(function(){

this.__proto__ = draggable;

var that = this;

this.__proto__.startDragMode = function(ev){

var evt = ev || window.event;
var target = evt.target || evt.srcElement;
target = target.parentNode;

...

}).apply(toolbarHeader);

I want my toolbarHeader's startDragMode method to override the parent method, and inherit the rest of the parent's methods.

Is this how I should be using inheritance? Is this a good design? Please keep in mind that the module design pattern was chosen for extensibility and to avoid the hassle of namespacing and instantiation.

I thought I just leave a general review if you don't mind.

MDN has this to say about __proto__:

  Warning: While Object.prototype.__proto__ is supported today in most browsers, its existence and exact behavior has only been standardized in the ECMAScript 6 specification as a legacy feature to ensure compatibility for web browsers. For better support, it is recommended that only Object.getPrototypeOf() be used instead.

The names of some parameters are somewhat short.

function(eH)
function(ev)
function(cnv)

The members of the class that they are modifiying however, have long and descriptive names:

var eventHandler
var canvas

Classes are made to be reused. As a user of your class, I don't really care about how you name your internal variables or if they even exist. But I'd be really happy to see some readable self documenting method parameter names show up in my auto completion.

setPosition = function(dX, dY)

Imo, if I want to set the position, I'd expect the parameters to be x and y. If they start with a d I'd expect some difference in position as the behaviour of the function. The method name implies an absolute position, but the parameter names imply a relative one.


Concatenate two arrays generated randomly I have written the code to concatenate two arrays . Please let me know of any loopholes in the written code.

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

public class ConcatArr {
    static Random rand = new Random();
    static Scanner in = new Scanner(System.in);
    public static void main(String[] args){
        System.out.println("Main Method Started");
        System.out.println("Initialize the 1st array");
        int[] arr1 = initArr();
        System.out.println("The element of the 1st array");
        print(arr1);
        System.out.println("Initialize the 1st array");
        int [] arr2 = initArr();
        System.out.println("The element of the 2nd array");
        print(arr2);
        int[] contArr = concatArr(arr1,arr2);
        System.out.println("Concatnated array");
        print(contArr);
        System.out.println("Main Method Ended");
    }
    public static void print(int[] arr){
        System.out.print("{");
        for(int val:arr){
            System.out.print(val + "  ");
        }
        System.out.print("}");
        System.out.println();
    }
    public static int[] initArr(){
        System.out.println("Enter the limit of the array");
        int limit = in.nextInt();
        System.out.println("Enter the maximum value of the array");
        int maxValue = in.nextInt();
        int[] arr = new int[limit];
        for(int i=0;i<limit;i++){
            arr[i] = rand.nextInt(maxValue);
        }
        return arr;
    }
    public static int[] concatArr(int[] arr1,int[] arr2){
        int[] tempArr = new int[arr1.length+arr2.length];
        for(int i=0;i<arr1.length;i++){
            tempArr[i] = arr1[i];
        }
        for(int j=0;j<arr2.length;j++){
            tempArr[j+arr1.length] = arr2[j];
        }
        return tempArr;
    }
}

Use the native method arraycopy as opposed to manually copying the elements for your concatenation method for better performance:

public static int[] concat(int[] a, int[] b) {
   int aLen = a.length;
   int bLen = b.length;
   int[] c= new int[aLen+bLen];
   System.arraycopy(a, 0, c, 0, aLen);
   System.arraycopy(b, 0, c, aLen, bLen);
   return c;
}

Also think about making more use of enhanced for loops, how come you are only using them for part of your solution and not the rest?

for(int i:arr){
    i = rand.nextInt(maxValue);
}


NSManagedObject in need of smart setters I'm just beginning my foray into iOS development and need a code review.  I have a custom NSManagedObject that needs some smart setters.  I'm new to Objective-C, iOS, and core-data development, so I don't trust myself yet.  I come from a Java background, so smart setters are what I know, but might not be best when using Core Data.

+(void)initialize
{
    scoredKeys = @[@"fullSwings", @"putts", @"fairwayBunkers", @"greenBunkers", @"chipsOrPitches", @"rescue", @"wastedOrPenalty"];
}

-(id) initWithHoleNumber:(int)holeNumber
{
    self = [super init];
    if (self) {
        [self setHoleNumber:[NSNumber numberWithInt:holeNumber]];
    }
    return self;
}

-(void) incrementItem:(NSString *)key
{
    [self willAccessValueForKey:key];
    NSNumber *value = [self primitiveValueForKey:key];
    int intValue = value.intValue;
    [self didAccessValueForKey:key];

    [self willChangeValueForKey:key];
    [self setPrimitiveValue:[NSNumber numberWithInt: intValue++] forKey:key];
    [self didChangeValueForKey:key];

    [self recalculateScore];
}

-(void) recalculateScore
{
    NSString *key;
    NSNumber *current;
    int tally = 0;

    for (key in scoredKeys) {
        [self willAccessValueForKey:key];
        current = [self primitiveValueForKey:key];
        tally = tally + current.intValue;
        [self didAccessValueForKey:key];
    }

    [self willChangeValueForKey:@"score"];
    [self setPrimitiveValue:[NSNumber numberWithInt:tally] forKey:@"score"];
    [self didChangeValueForKey:@"score"];
}

I am also seeking general comments about my approach.

In the core-data docs, it says the auto generated accessors are much faster than anything a developer can write, so is this a good idea?
Should I not even have a score property, and just have a method that returns the tally?

without referring to your code, I think there is one pattern, that makes core data more fun but sadly isnt supported by Xcode out-of-the-box:

Have two classes for each model: one for automatic code generation — it will be jsut changed by Xcode — and on that inherits from the first, that can be changed and will never touched again by the machine after creation.

// for the machine
@class _Event : NSManagedObject
@end

// for human beings
@class Event : _Event
// what ever you need: put it here
@end

a great tool to teach Xcode this pattern is mogenerator. mogenerator + Xcode4.x

Another similar approach would be to have categories on the Model class. But this is less powerful and needs more attention for importing.

Now to your code.

I dont see any reason to overwrite the setter. if apple says that you most likely will never be able to write as performant accessors as the provide, they probably are right. Instead you should create your own method, that will set up a object as need. you could overwrite save and similar methods, or access it via a wrapper class, that will set it up for you.

  I plan to create a service class as a factory that manages the entities and their interactions with the context.

that sounds valid and could be the wrapper class mentioned above. But make sure, you create the model objects with initWithEntity:insertIntoManagedObjectContext:, as you want them in your context.

Core Data is a great and powerful framework. but beware: if you shoot yourself in the foot, most likely both your legs will be gone.
2 Years ago I was working in a team where the Engineer responsible for persistency accidentally overwrote an internal method of NSManagedObject. It took him one week to find the cause and fix it by renaming his method. This line of code was probably the most expensive in the whole project. There-for another hint: prefix your custom methods.


Is this good practice with unit-testing? I've created unit test for the "student" CRUD operations that looks like this:

[Test]
    public void Can_Exec_CrudOps()
    {
        #region Prepare

        var account = Processor.Execute(new CreateAccount
        {
            Email = "email@example.com",
            Username = "username",
            Password = "password",
        }).Result;

        #endregion

        var student = Processor.Execute(new CreateStudent
        {
            AccountId = account.EntityID
        }).Result;

        Assert.NotNull(student);
        student.AreEqual(Processor.Execute(new GetStudent
        {
            EntityId = student.EntityID
        }).Result);
        student.AreEqual(Processor.Execute(new GetStudentByAccount
        {
            AccountId = account.EntityID
        }).Result);

        var upd_student = Processor.Execute(new UpdateStudent
        {
            EntityId = student.EntityID,
            Flags = StudentFlags.Active
        }).Result;

        Assert.NotNull(upd_student);
        upd_student.AreEqual(student);
        upd_student.Flags.AreEqual(StudentFlags.Active);

        Processor.Execute(new DeleteStudent
        {
            EntityId = student.EntityID
        });
    }

As you can see, the region #prepare contains code that will insert required data to the database for the current test to be possible.

Is it good practice to create a single test for each CRUD operation and then call that test inside the tests that require them?

What I'm thinking about would look like this:

 [TestFixture]
public class StudentTest : OperationTest
{
    protected override IEnumerable<Type> ModelMappings
    {
        get
        {
            return NHMappings.EDUCATION;
        }
    }

    [Test]
    public void Can_Exec_CrudOps()
    {
        #region Prepare
        // here is what changed //
        var acc = new AccountTests();
        var acc_id=    acc.create_account();

        #endregion

        var student = Processor.Execute(new CreateStudent
        {
                        //here too//
            AccountId = acc_id
        }).Result;

        Assert.NotNull(student);
        student.AreEqual(Processor.Execute(new GetStudent
        {
            EntityId = student.EntityID
        }).Result);
        student.AreEqual(Processor.Execute(new GetStudentByAccount
        {
                        // and here//
            AccountId = acc_id
        }).Result);

        var upd_student = Processor.Execute(new UpdateStudent
        {
            EntityId = student.EntityID,
            Flags = StudentFlags.Active
        }).Result;

        Assert.NotNull(upd_student);
        upd_student.AreEqual(student);
        upd_student.Flags.AreEqual(StudentFlags.Active);

        Processor.Execute(new DeleteStudent
        {
            EntityId = student.EntityID
        });
        // and here //
        acc.deleteaccount(acc_id);
    }
}

So the point is not to write the same code over and over again.

No, I would say, and the reason for that is that you have too many asserts in your test (it does to many thinks). There should only be one reason for a test to fail. So:

Split your test into smaller ones with a more descriptive name and stick to one assert per test. By doing so it's also easier to track down why a test is failing, both in the case is the bug is in the test or in the production code.


Get the first two numbers of a string What would be the best way to get the first two numbers of a string?

I can do it like this:

public static String getFirstTwoNumbersOfString(String s){
        if(s == null)
            return "";

        String numbers = "0123456789";

        String result = "";

        for(int i=0; i<s.length() && result.length()<2; i++){
            String c = String.valueOf(s.charAt(i));
            if(numbers.contains(c)){
                result += c;
            }
        }

        return result;
    }

But I think there must be a better way, more concise and with better performance.

In fact, you can:

public static String getFirstTwoNumbersOfString2(String s) {
    StringBuilder sb = new StringBuilder(2); // Capacity of 2 chars.

    for (char c : s.toCharArray()) {
        if (Character.isDigit(c)) {
            sb.append(c);

            if (sb.length() == 2) {
                break;
            }
        }
    }

    return sb.toString();
}

Above you can nicely ask Character whether a given character is a digit. Also, StringBuilder.append runs in constant amortized time unlike string concatenation which will build a copy of two concatenated strings.

Hope that helps.