Is this a correct recursive Scala iterator creation? I am diving into Scala (using 2.10) headfirst and am trying out a simple task of taking bulk data and then doing something with that stream. The first part is generating that stream. I have seem to be able to cobble together what seems to work, but I think I am missing shortcuts in scala. Any pointers or code cleanup recommendations are welcome! I feel like getInnerPolygons could be simplified using a match statement, but I haven't gotten my head around those yet.

So, the process: With gdal, you retrieve a dataset, then retrieve the layer. From the layer, you can iterate over the features. From those features, you can get the geometries from each feature. Now, most features are just one polygon. If it is, take the outer ring of that polygon and pass it up the line. If the polygon is multi-part (think the hawaii islands as one "feature" where each island is a polygon), retrieve the polygons, then split that into their individual outer rings (this is that recursive call into getInnerPolygons).

Again: any Scala best practices or shortcuts that I missed. I would love to hear about.

def getInnerPolygons(geometry: Geometry) : Iterator[Geometry] = {
    Iterator
        .range(0,geometry.GetGeometryCount())
        .flatMap(i=>{
            val innerGeometry = geometry.GetGeometryRef(i)
            if(innerGeometry.GetGeometryName().equals("LINEARRING"))
                Iterator.single(innerGeometry)
            else
                getInnerPolygons(innerGeometry)
       })
}

def getPolygons(layer : Layer) : Iterator[Geometry] = {
    Iterator.continually[Feature](layer.GetNextFeature())
        .takeWhile(_!=null)
        .flatMap(feature=>getInnerPolygons(feature.GetGeometryRef()))
}

def main(args: Array[String]){
    ogr.RegisterAll()

    val dataSet = ogr.Open("myshapefile.shp")
    val layer = dataSet.GetLayer(0)

    for(i <- getPolygons(layer)) println(i.GetGeometryName())

    println("Done")
}

micro changes:
use == instead of equals because "==" means "equals".

Scala does not follow the Java convention of prepending set/get to mutator and accessor methods.

Try to split different parts of work (abstraction)

def innerPolygons(geometry: Geometry) : Iterator[Geometry] =
  innerGeometry(geometry).flatMap(inner)

def innerGeometry(geometry: Geometry): Iterator[Geometry]=
  (0 to geometry.GetGeometryCount()).map(geometry.GetGeometryRef(_))

def inner:Geometry => Iterator = g => if ("LINEARRING"== g.GetGeometryName())
  Iterator.single(g)
 else
  innerPolygons(g)


Simple Binary Search Tree Class with Insert and Search Functions I am trying to learn to implement DSs in C++ and here is a simple implementation. Please review in terms of optimization, memory management, coding standards, etc

#include <stdio.h>
#include <iostream>

class BstNode{
int data;
BstNode* left;
BstNode* right;

public:
BstNode(int data)
{
    this->data = data;
    this->left = NULL;
    this->right = NULL;
}
~BstNode();

void Insert(int data)
{
    if(this->data >= data)
    {
        if (this->left == NULL)
            this->left = new BstNode(data);
        else
            this->left->Insert(data);
    }
    else
    {
        if (this->right == NULL)
            this->right = new BstNode(data);
        else
            this->right->Insert(data);
    }
}

bool Search(int data)
{
    if(this->data == data)
        return true;
    else if(this->data >= data)
    {
        if(this->left == NULL)
            return false;
        else
            return this->left->Search(data);
    }
    else
    {
        if(this->right == NULL)
            return false;
        else
            return this->right->Search(data);
    }

}
};

int main()
{
BstNode* ptr_root = new BstNode(15);
ptr_root->Insert(10);
ptr_root->Insert(16);
int num;
std::cout<<"Enter the number: \n";
std::cin>> num;
if (ptr_root->Search(num))
    std::cout<<"Found\n";
else
    std::cout<<"Not Found\n";

return 0;
}

Overall organization

Right now, you've defined a BstNode, which you use directly in main. I'd at least consider defining a Bst class, and then (probably inside that) a Node class.

class Bst {
    class Node {
        // only code to deal with individual nodes goes here
    };

    Node *root;
public:

    bool insert(int value);

    bool find(int value);
};

Then to create a Binary search tree, you'd just instantiate a Bst object, and use it:

int main() {

    Bst tree;

    // oops--this will lead to a completely imbalanced tree.
    for (int i=0; i<10; i++)
        tree.insert(i);

}

in-class initialization

Since you (almost?) always create a node with its child pointers set to nullptrs, I'd just specify that directly in the class instead of in the ctor:

// ...
class Node {
    int value;
    Node *left = nullptr;
    Node *right = nullptr;

    Node(int value) : value(value) {}
};

Insertion and Search

I'd at least consider whether you really want to use recursion to do insertion and/or search. Since you only ever descend to one child of any given node, it's pretty easy to use iteration for these tasks.

Insert return type

I'd at least consider having Insert return a bool to indicate whether it actually inserted a value in the tree or not (e.g., return false if a value is already present).

Collection type

What you've currently implemented is roughly equivalent to a std::set. That is, it stores only keys. This can certainly be useful, but there are times its also useful to have data associated with the key (i.e., roughly equivalent to std::map).

Generic Code

Right now, you've written the code so the only thing can be stored in your BST is ints. I'd at least consider making it generic so other types can be stored.

template <class T, class Cmp = std::less<T>>
class Bst {
    // ...

This increases complexity a little, but increases functionality quite a lot.


Generic Doubly Linked-list implementation in C++ I am just learning about C++ templates and generic types, I decided it would be nice to try to create a generic container class as a challenge and test my knowledge in the process. Here is what I have come up with.

#include <iostream>
#include <cstdlib>
#include <cassert>

template <typename __T>

class List{
private:
    struct node{
        __T data;
        node* next;
        node* prev;
    };
    typedef long long unsigned int c_size;
    node* head;
    node* tail;
    node* current;
    c_size m_S;

public:
    List()
        :head{NULL},tail{NULL},current{NULL},m_S{0}
    {}

    List(std::initializer_list<__T> __l)
        :head{NULL},tail{NULL},current{NULL},m_S{0}
    {
        for(auto const& elem:__l){
            insert_back(elem);
        }
    }
    void insert_front(__T _d){
        ++m_S;
        node* new_node{new node};
        new_node->data = _d;
        new_node->next = head;
        new_node->prev = NULL;

        if (head != NULL){
            head->prev = new_node;
        }
        else
        {
            tail = new_node;
        }
        head = new_node;

    }

    void print(){
        if (m_S == 0) return;
        node *current = head;
        while (current->next != NULL){
            std::cout << current->data << ", ";
            current = current->next;
        }
        std::cout << current->data << '\n';
    }

    void insert_back(__T _d){
        ++m_S;
        node* new_node = new node;
        new_node->data = _d;
        new_node->next = NULL;
        node* last = head;

        if (head == NULL) {
            new_node->prev = NULL;
            head = new_node;
            tail = new_node;
        }
        else{
            tail->next = new_node;
            new_node->prev = tail;
            tail = new_node;
        }
    }

    constexpr __T first(){
        assert(head != NULL && "Calling .front() on an empty list");
        return head->data;
    }
    constexpr __T last(){
        assert(tail != NULL && "Calling. last() on an empty list");
        return tail->data;
    }

    constexpr c_size size(){return m_S;}

    void remove_all(){
        current = head;
        while(current->next != NULL){
            current = current->next;
            delete current;
        }
        head = NULL;
        tail = NULL;
        current = NULL;
    }

};

Thanks in advance !

Identifiers that start with a double underscore, or a single underscore followed by a capital letter, are reserved for use by the implementation. Your use of __T could conflict with this use.
Spacing is important. The blank line between template <typename __T> and the following class List line makes it harder to recognize the class as a template class.
What is the purpose of the current member? It is unnecessary.
c_size should probably be std::size_t, not unsigned long long. Because it is a private typedef, users of your class cannot access the type and can only declare a variable to hold the returned value using auto.
You don't declare the copy constructor, copy assignment operator, move constructor, nor move assignment operators. This will cause problems with assignments of one List object to another.
insert_head and insert_back are very similar functions, but have been written somewhat differently. The implementation for insert_back can be made shorter by making it similar to insert_head. In particular, the assignment to new_node->prev can be done before the if (using new_node->prev = tail;, because when head is NULL, tail should also be), while the common tail = new_node; can be done after.
first, last, and size should also have the const modifier (e.g., constexpr c_size size() const.
remove_all will crash because you access current->next after delete current, nor do you delete the first (head) element.


Method Calculate User Answers I am writing spring hibernate app to make online test and this method for calculating user grade I have one to many relations between multi-choice question and answer.

Also, please check this question

@Service
public class StudentServiceImpl implements StudentService {

    private Logger logger = LoggerFactory.getLogger(this.getClass());

    @Autowired
    QuestionRepository questionRepository;

    @Autowired
    UserRepository userRepository;

    @Autowired
    Transformer transformer;

    @Override
    @Transactional
    public void correctAnswer(List<StudentAnswersDTO> questionAnswers) {
        logger.info("****** start correct answers ********");
        Long grade = 0L;
        List<QuestionRightAnswerDTO> rightAnswerDTOS = questionRepository.getRightAnswers();
        for (StudentAnswersDTO questionAnswer : questionAnswers) {
            if (questionAnswer.getType().toLowerCase().equals(AppConstants.MULTI_CHOICE)) {
                if (isCorrectAnswer(questionAnswer.getId(), questionAnswer.getAnswerId(), rightAnswerDTOS))
                    grade++;
            }
        }

        User currentStudent = userRepository.findByMobileNumber(SecurityHelper.getCurrentUser());
        ((Student) currentStudent).setGrade(grade);
        ((Student) currentStudent).setStudentAnswers(transformer.transform(questionAnswers, AnswerSelected.class));

        logger.info("****** end correct answers ********");
    }

    private boolean isCorrectAnswer(Long id, Long answerId, final List<QuestionRightAnswerDTO> rightAnswerDTOS) {
        for (QuestionRightAnswerDTO rightAnswer : rightAnswerDTOS) {
            if (id.equals(rightAnswer.getQuestionId()) && answerId.equals(rightAnswer.getAnswerId()))
                return true;
        }
        return false;
    }
}

Fist of all I will recommend you to use Constructor based dependency injection. Then I can not see the relation between the Question, QuestionAnswer, and QuestionRightAnswer entities... currently you are selecting all the right answers from the DB which you should NOT do! You have to get all the correct answers for this question only. Use the question.id from QuestionAnswer just to match the QuestionRightAnswer.questionId FK. Also you do take care of the MULTI_CHOICE questions only, but why?

Consider an implementation such as:

@Service
public class StudentServiceImpl implements StudentService {

    private Logger logger = LoggerFactory.getLogger(this.getClass());

    QuestionRepository questionRepository;
    UserRepository userRepository;
    Transformer transformer;

    @Autowired
    public NotificationService(QuestionRepository questionRepository,
                               UserRepository userRepository,
                               Transformer transformer) {
        this.questionRepository = questionRepository;
        this.userRepository = userRepository;
        this.transformer = transformer;
    }

    @Override
    @Transactional
    public void correctAnswer(List<StudentAnswersDTO> questionAnswers) {
        logger.info("****** start correct answers ********");
        Long grade = questionAnswers
            .stream()
            .filter(questionAnswer -> questionAnswer.getType().toLowerCase().equals(AppConstants.MULTI_CHOICE))
            .filter(questionAnswer -> questionRepository
                .getRightAnswersByQuestionId(questionAnswer.getId())
                .stream()
                .anyMatch(rightAnswer -> questionAnswer.getAnswerId().equals(rightAnswer.getAnswerId())))
            .count();

        User currentStudent = userRepository.findByMobileNumber(SecurityHelper.getCurrentUser());
        ((Student) currentStudent).setGrade(grade);
        ((Student) currentStudent).setStudentAnswers(transformer.transform(questionAnswers, AnswerSelected.class));
    }
}


Optimizing nested "Select Distinct" query I am using the following query which is causing issues. Note following points:

I am using 4 tables in this query:

message_share
messages
myusertable
friends

What I am trying to do in this query is to display messages, and share messages of users who are friend with each other.

This is similar to Facebook wall where they display friends' wall data.

The problem is only myusertable is causing the issue. It increase the load too much,  even to where the query can timeout.

When I remove myusertable from this query then it works fine, the use of myusertable is to fetch user full name. Everything else we are doing with userid and does not need  myusertable. For the sole purpose of getting user full name are we using myusertable.

The query is

(SELECT DISTINCT M.msg_id, M.uid_fk, M.message, S.created, M.like_count,M.comment_count,M.share_count, U.username,M.uploads, S.uid_fk AS share_uid,S.ouid_fk AS share_ouid
FROM friends F
LEFT JOIN message_share S ON S.ouid_fk <> F.friend_two
LEFT JOIN messages M ON M.msg_id = S.msg_id_fk AND M.uid_fk = S.ouid_fk
LEFT JOIN myusertable U ON U.uid = M.uid_fk AND U.status1='1'
WHERE F.friend_one='199095' AND F.role='fri'
GROUP BY msg_id
ORDER BY created DESC LIMIT 10)
UNION
(SELECT DISTINCT M.msg_id, M.uid_fk, M.message, M.created, M.like_count,M.comment_count,M.share_count, U.username,M.uploads, '0' AS share_uid, '0' AS share_ouid
FROM friends F
LEFT JOIN messages M ON M.uid_fk = F.friend_two
LEFT JOIN myusertable U ON U.uid = M.uid_fk AND U.status1='1'
WHERE F.friend_one='199095'
GROUP BY msg_id
ORDER BY created DESC LIMIT 10)

Is there anything that I can do with myusertable to make this query faster?

Nitpicking
Aliases/notation
Single-letter alias names are not very helpful. Aliases should at least give Mr. Maintainer a clue what it stands for. What if you had a table called family along with friends... would you call it F2?
FROM friends F
LEFT JOIN message_share S ON S.ouid_fk <> F.friend_two
LEFT JOIN messages M ON M.msg_id = S.msg_id_fk AND M.uid_fk = S.ouid_fk
LEFT JOIN myusertable U ON U.uid = M.uid_fk AND U.status1='1'

Also, I find it good practice to use the optional AS keyword, just looks better in my opinion. How about:
FROM friends AS Frnd
LEFT JOIN message_share AS Share ON Share.ouid_fk <> Frnd.friend_two
LEFT JOIN messages AS Msg ON Msg.msg_id = Share.msg_id_fk AND Msg.uid_fk = Share.ouid_fk
LEFT JOIN myusertable AS User ON User.uid = Msg.uid_fk AND Usr.status1 = '1'

Column names
Some of your column names are pretty cryptic. I realize you may not have any say into it, but if this is your database you may consider changing names like ouid_fk and such to something more meaningful, if a bit more verbose.
Indentation/spacing
Use line breaks and indentation to facilitate reading of your queries. Also consistent spacing is an improvement.
(SELECT DISTINCT M.msg_id, M.uid_fk, M.message, S.created, M.like_count,M.comment_count,M.share_count, U.username,M.uploads, S.uid_fk AS share_uid,S.ouid_fk AS share_ouid

Why not instead:
(SELECT DISTINCT
  Msg.msg_id,
  Msg.uid_fk,
  Msg.message,
  Share.created,
  Msg.like_count,
  Msg.comment_count,
  Msg.share_count,
  Usr.username,
  Msg.uploads,
  Share.uid_fk AS share_uid,
  Share.ouid_fk AS share_ouid

INT in a VARCHAR column?
I find this odd:
AND Usr.status1 = '1'

To me in a well-designed database, this status1 column should be int (4 bytes), smallint (2 bytes) or tinyint (1 byte). Not to mention, status1 is not a very good column name.
SELECT DISTINCT
I don't feel that this is needed at all, for a couple of reasons:

Do you expect multiple messages to have the same msg_id? Wouldn't that completely defeat the purpose of an ID?

Your GROUP BY msg_id already does this, but it does the work on the result set rather than the source data set, which will likely be much faster.

Here is what I would refactor to. However there is no way for me to test performance since you did not provide DDL or sample data.
(SELECT
  Msg.msg_id,
  Msg.uid_fk,
  Msg.message,
  Share.created,
  Msg.like_count,
  Msg.comment_count,
  Msg.share_count,
  Usr.username,
  Msg.uploads,
  Share.uid_fk AS share_uid,
  Share.ouid_fk AS share_ouid
FROM friends AS Frnd
LEFT JOIN message_share AS Share ON Share.ouid_fk <> Frnd.friend_two
LEFT JOIN messages AS Msg ON Msg.msg_id = Share.msg_id_fk AND Msg.uid_fk = Share.ouid_fk
LEFT JOIN myusertable AS User ON User.uid = Msg.uid_fk AND Usr.status1 = '1'
WHERE Frnd.friend_one='199095' AND Frnd.role='fri'
GROUP BY Msg.msg_id
ORDER BY Share.created DESC LIMIT 10)
UNION
(SELECT
  Msg.msg_id,
  Msg.uid_fk,
  Msg.message,
  Share.created,
  Msg.like_count,
  Msg.comment_count,
  Msg.share_count,
  Usr.username,
  Msg.uploads,
  Share.uid_fk AS share_uid,
  Share.ouid_fk AS share_ouid
FROM friends AS Frnd
LEFT JOIN message_share AS Share ON Share.ouid_fk <> Frnd.friend_two
LEFT JOIN messages AS Msg ON Msg.msg_id = Share.msg_id_fk AND Msg.uid_fk = Share.ouid_fk
LEFT JOIN myusertable AS User ON User.uid = Msg.uid_fk AND Usr.status1 = '1'
WHERE Frnd.friend_one='199095'
GROUP BY Msg.msg_id
ORDER BY Share.created DESC LIMIT 10)


Calculating and printing a credit card balance I am working on a problem that simply calculates and prints the credit card balance. I hope this code can be improved/optimized a lot.

I am from a Java background. If it was Java code, I would have pulled all the code lines inside the for loop and place it at the beginning of the method and inside for loop I would call the method recursively. I tried to do the same in python but it hit me with some error:

  RuntimeError: maximum recursion depth exceeded while calling a Python object

From a quick internet search I found that it is better to go for iteration instead of recursion:

'''
Program to calculate credit card balance after 1 year
'''
def calculateCreditCardBalance(balance, annualInterestRate, monthlyPaymentRate):

    totalPaid = 0

    for thisMonth in range(1,13):
        monthlyInterestRate = annualInterestRate / 12.0
        minimumMonthlyPayment = round(monthlyPaymentRate * balance, 2)
        monthlyUnpaidBalance = balance - minimumMonthlyPayment
        updatedBalanceEachMonth = round(monthlyUnpaidBalance + (monthlyInterestRate * monthlyUnpaidBalance) , 2)

        print 'Month: %d' %(thisMonth)
        print 'Minimum monthly payment: ' + str(minimumMonthlyPayment)
        print 'Remaining balance: ' + str(updatedBalanceEachMonth)

        balance = updatedBalanceEachMonth
        totalPaid += minimumMonthlyPayment

    print 'Total paid: ' + str(totalPaid)
    print 'Remaining balance: '+ str(balance)

calculateCreditCardBalance(4842, 0.2, 0.04)

A good thing you can do is to create more functions, which return values in the dictionary form.

Pseudo-code:

def calculateCreditCardBalance(balance, annual_interest_rate, monthly_payment_rate):
    total_paid = 0
    curr_values = ["balance" => balance, "annual_interest_rate" => annual_interest_rate, "monthly_payment_rate" => monthly_payment_rate]
    foreach this_month in rage(1,13):
        month_values = calculate_month_values(curr_values)
        print_values(month_values)
        update_month_values(month_values,curr_values)
    print 'Total paid: ' + str(curr_values["total_paid"])
    print 'Remaining balance: '+ str(curr_values["total_paid"])


Java builder for user contact information, with optionals I have this simple builder here:

public class User {

    private final String name;
    private final String lastname;
    private final String phone;

    private User(Builder builder) {
        this.name = builder.name;
        this.lastname = builder.lastname;
        this.phone = builder.phone;
    }

    public String getName() {
        return name;
    }

    public String getLastname() {
        return lastname;
    }

    public String getPhone() {
        return phone;
    }

    public static class Builder {
        private String name;
        private String lastname;
        private String phone = "";

        public Builder(String name, String lastname) {
            name(name);
            lastname(lastname);
        }

        private Builder name(String name) {
            this.name = Optional.ofNullable(name).orElseThrow(() -> new IllegalArgumentException("Name must not be null"));
            return this;
        }

        private Builder lastname(String lastname) {
            this.lastname = Optional.ofNullable(lastname).orElseThrow(() -> new IllegalArgumentException("Last name must not be null"));
            return this;
        }

        public Builder phone(String phone) {
            this.phone = Optional.ofNullable(phone).orElse("");
            return this;
        }

        public User build() {
            return new User(this);
        }

    }
}

This code works as expected but I have some doubts. My question is about private methods in builder (name and lastname). Is it allowed in a world of a clean code? And also about this Optionals, are they well written? I just started using this functionality so I don't know for sure if I am right here.

My understanding from a builder is a bit different. In my opinition, the User should not have a dependency to the Builder (you have a bidirectional dependency which is usually bad anyway). With that said, use the User as member variable within your builder:

public class UserBuilder {
  private User user = new User();

  public UserBuilder withUsername(String username) {
    user.setUsername(username);
  }

  public User build() {
    return user;
  }
}

I also prefer a verb in my method names (they should have, actally, but in that case I think it's okayish), that's why I use 'with' as prefix; And I explicitly do not use not 'set' as prefix, to distinguish it from the setter of the User type.

If you do it like that, the 'actual building' of the user is within the builder and not in the User - and the User type is kept quite stupid.

It might seem a bit strange, that the 'build' method doesn't actually build, but non-persistent built types are often only used for unit testing. The sweet part about builders, you can expand them to persist your built types or other fancy requirements.


Faster way of reading csv to grid I have following in Windows Forms .NET 3.5

It works fine for csv with records less than 10,000 but is slower for records above 30,000.
Input csv  file can can any records between 1 - 1,00,000 records

Code currently used :

/// <summary>
        /// This will import file to the collection object
        /// </summary>
        private bool ImportFile()
        {
            try
            {

                String fName;
                String textLine = string.Empty;
                String[] splitLine;

                // clear the grid view

                accountsDataGridView.Rows.Clear();

                fName = openFileDialog1.FileName;

                if (System.IO.File.Exists(fName))
                {
                    System.IO.StreamReader objReader = new System.IO.StreamReader(fName);

                    do
                    {
                        textLine = objReader.ReadLine();
                        if (textLine != "")
                        {
                            splitLine = textLine.Split(',');
                            if (splitLine[0] != "" || splitLine[1] != "")
                            {
                                accountsDataGridView.Rows.Add(splitLine);
                            }
                        }
                    } while (objReader.Peek() != -1);
                }
                return true;
            }
            catch (Exception ex)
            {
                if (ex.Message.Contains("The process cannot access the file"))
                {
                    MessageBox.Show("The file you are importing is open.", "Import Account", MessageBoxButtons.OK, MessageBoxIcon.Warning);
                }
                else
                {
                    MessageBox.Show(ex.Message);
                }

                return false;
            }

        }

Sample Input file :

18906,Y
18908,Y
18909,Y
18910,Y
18912,N
18913,N

Need some advice on optimizing this code for fast reads & view in grid.

What's the use of presenting even 5,000 records to the user? Do you expect a user to scroll to row 4711, look at "18912,N", and say "Strange, why wasn't there a '12345,N' in row 1234?", or "Interesting, now let's see where the corresponding '54321,X" is to be found!"?

Your GUI should help the user to specify the comparatively small set(s) of records pertinent to his current problem/question; even if working with the data means "check every item in turn", it would be more human to present the records in small batches.

All this points to using your file as a database table bound to the controls of your GUI; then the problem of "reading a .csv fast" completely vanishes.


Returning JSON depending on the route_id I have a controller that has a purpose of returning json depending on the route_id being passed from the route (Using laravel). There's a point in my code where I resorted to using a double nested foreach loop in order to iterate the values I want to be stored into an array. Everything works fine, but I feel like the code is messy. Any tips?

public function getRoute($id)
{
    $route = Route_log::find($id);
    $allWayPoints = explode(";", $route->route_order);
    $firstWaypoint = array_shift($allWayPoints); //get first waypoint

    foreach($allWayPoints as $allWayPoint)
        {
            $allOrdersInWaypoint = Transaction::where('waypoint_id', "=", $allWayPoint)->where('type', "=", "DD")->get()->lists('order_id');
            $allOrders[] = array($allOrdersInWaypoint);
        }
  ///**THIS PART**
    foreach($allOrders as $allOrder)
    {
        foreach($allOrder as $aO)
        {
            foreach($aO as $a)
            {
                $otherOrders = Order::find($a);

                if(isset($otherOrders))
                {
                $getOtherOrders[] = array(
                                            'id'             => $otherOrders->id,
                                            'tracking_id'    => $otherOrders->tracking_id,
                                            'shipper_ref_no' => $otherOrders->shipper_ref_no);
                }
            }
        }
    }

    //get all transactions within the first way point
    $ordersInWaypoint = array();
    $ordersInWaypoint = Transaction::where('waypoint_id', "=", $firstWaypoint)->where('type', "=", "PP")->get()->lists('order_id');

    //get order id, tracking_id, and shipper ref no and store into an array
    foreach($ordersInWaypoint as $ordersInWaypoints)
        {
            $order = Order::find($ordersInWaypoints);

            //this array stores id, trackingId, and shipper_ref_no per order
            $getOrder[] = array(
                                'id'             => $order->id,
                                'tracking_id'    => $order->tracking_id,
                                'shipper_ref_no' => $order->shipper_ref_no);
        }

    $json = (array('first_waypoint' => $getOrder, 'remaining_waypoints' => $getOtherOrders));

    return $json;
}

}

To show an example array inside $allOrders:

 array (size=20)
   0 =>
     array (size=1)
      0 =>
         array (size=2)
           0 => int 491
           1 => int 494
   1 =>
     array (size=1)
       0 =>
         array (size=0)
           empty
   2 =>
     array (size=1)
       0 =>
         array (size=1)
           0 => int 498
   3 =>
     array (size=1)
       0 =>
         array (size=2)
           0 => int 487
           1 => int 493
   4 =>
     array (size=1)
       0 =>
         array (size=1)
           0 => int 503
   5 =>
     array (size=1)
       0 =>
         array (size=1)
           0 => int 480
   6 =>
     array (size=1)
       0 =>
         array (size=1)
           0 => int 476
   7 =>
     array (size=1)
       0 =>
         array (size=1)
           0 => int 482
   8 =>
     array (size=1)
       0 =>
         array (size=1)
           0 => int 515

How about somethink like:

 $allOrders = array();
 foreach($allWayPoints as $allWayPoint)
 {
            $allOrdersInWaypoint = Transaction::where('waypoint_id', "=", $allWayPoint)->where('type', "=", "DD")->get()->lists('order_id');
 //           $allOrders[] = array($allOrdersInWaypoint);
            $allOrders = array_merge($allOrders, $allOrdersInWaypoint);
 }

 foreach($allOrders as $order)
 {
       $otherOrders = Order::find($order);

       if(isset($otherOrders))
       {
            $getOtherOrders[] = array(
                                            'id'             => $otherOrders->id,
                                            'tracking_id'    => $otherOrders->tracking_id,
                                            'shipper_ref_no' => $otherOrders->shipper_ref_no);
       }
 }

Then they wont be nested and $allOrders will simply be a flat array containing the numbers, assuming $allOrdersInWaypoint is a array of numbers.


LINQ statement to aggregate data Teachers can raise 'dramas' which are when students forget equipment, don't do homework or are flagged for concern. Each drama for each student is stored as a separate record.

I seek a list of all unresolved dramas, grouped by student & date. So if on one day a student forgets their homework and doesn't have equipment, they come up as a single record with the properties EquipmentDrama and HomeworkDrama both set to true.

var dramas = ctx.Dramas.Where(o => !o.Resolved).Select(o => new
  {
    o.Id,
    o.StudentUsername,
    o.Student.FirstName,
    o.Student.LastName,
    o.Student.TutorGroup,
    o.Student.ClassName,
    TeacherName = o.Teacher.FirstName + " " + o.Teacher.LastName,
    o.DramaType,
    o.DateForDetention,
    o.DateHappened
  }).ToLookup(o => o.StudentUsername + o.DateHappened.ToString("ddMMyy"), o => o);
var students = dramas.Select(o => o.Key).Distinct().Select(o => dramas[o].ToList()).Select(o => new FollowUpItem
  {
    Id = String.Join(",", o.Select(d => d.Id)),
    FirstName = o[0].FirstName,
    LastName = o[0].LastName,
    TutorGroup = o[0].TutorGroup,
    EquipmentDrama = o.Any(d => d.DramaType == DramaType.Equipment),
    HomeworkDrama = o.Any(d => d.DramaType == DramaType.Homework),
    IsFlagged = o.Any(d => d.DramaType == DramaType.Flagged),
    ClassName = o[0].ClassName,
    TeacherName = o[0].TeacherName,
    DateForDetention = o[0].DateForDetention,
    DateHappened = o[0].DateHappened
  }).ToList();

Note the reason for using an anonymous type in the first LINQ statement is that I want to retrieve the related fields o.Teacher.FirstName and o.Teacher.LastName in the initial (only) database call. There must be a better way of doing this though!

It seems like you could do this:

var results =
    (from d in ctx.Dramas
     where !d.Resolved
     group d by new { d.Student, DateHappened = d.DateHappened.ToString("ddMMyy") } into g
     select new
     {
         Student = g.Key.Student,
         DateHappened = g.Key.DateHappened,
         EquipmentDrama = g.Any(d => d.DramaType == DramaType.Equipment),
         HomeworkDrama = o.Any(d => d.DramaType == DramaType.Homework),
         IsFlagged = o.Any(d => d.DramaType == DramaType.Flagged),
         Dramas = g.Select(d => new
             {
                 d.Id,
                 TeacherName = d.Teacher.FirstName + " " + d.Teacher.LastName,
                 d.DateForDetention
             })
     });

NOTE: as far as I can tell, there's nothing limiting Teacher (or DateForDetention) to be the same in all grouped dramas; each drama could have a different Teacher. Here's an alternative that includes the teacher in the grouping:

var results =
    (from d in ctx.Dramas
     where !d.Resolved
     group d by new { d.Student, d.Teacher, DateHappened = d.DateHappened.ToString("ddMMyy") } into g
     select new
     {
         Student = g.Key.Student,
         DateHappened = g.Key.DateHappened,
         EquipmentDrama = g.Any(d => d.DramaType == DramaType.Equipment),
         HomeworkDrama = o.Any(d => d.DramaType == DramaType.Homework),
         IsFlagged = o.Any(d => d.DramaType == DramaType.Flagged),
         TeacherName = d.Key.Teacher.FirstName + " " + d.Key.Teacher.LastName,
         Dramas = g.Select(d => new
             {
                 d.Id,
                 d.DateForDetention
             })
     });

Of course, you could do the same for DateForDentention if you wanted.


Linux backup script in Python I've written a backup script to make a backup of the latest modified files in 12 hours. Basically, it searches two directories for modified files (statically coded) then the find command is used to find the related files. A log file is then created to keep a list of the files. Later on, they are used to form a list of files and these files are used for the backup by tar.

Let me know which parts could be developed so that I can improve on and learn new things.

#!/usr/bin/env python
''' backup script to make a backup of the files
    that are newer than 12 hours in the
    home directory
    see
    " man find "
    " man tar  "
    for more information on the dates and the specific options
    on how to set different specifications for the selections
    '''
import sys
import os
import time

if (len(sys.argv) != 2):
  # if argument count is different
  # than 2, then there is an error
  print "Error on usage -> binary <hours_count_back>"
  quit()
else:
  # convert to integer value
  hours_back = int(sys.argv[1])

message_start = "Operation is starting".center(50, '-')
message_end   = "Operation terminated with success".center(50,'-')
#
print message_start
# source directories to check for backup
source = [ '/home/utab/thesis', '/home/utab/Documents' ]
# find files newer than 24 hours
# change to the target directories
newer_files = []
#
log_file = "log.dat"
# cmd to find newer files
# than 24 hours
cmd_find = "find . -type f -a -mmin " + str(-hours_back*60) + " > " + log_file
for directory in source:
  # iterate over the directories
  # change to the directory first
  os.chdir(directory)
  # apply the command
  os.system(cmd_find);
  # files are found with respect to the current directory
  # change the "." to directory for correct backups
  c = 0
  # process log file
  files = []
  lines = []
  log_in = open(log_file,'r')
  # read lines without \n character
  while 1:
      l = log_in.readline()
      if not l:
        break
      # do not include the newline
      # -1 is for that
      lines.append(l[:-1])
  #
  for l in lines:
      l_dummy  = l.replace( '.', directory, 1 )
      files.append(l_dummy)
  # extend the list with newer files
  newer_files.extend(files)
#print newer_files
print newer_files
# date
today =  time.strftime('%Y%m%d')
# current time of the date
# possible to do different backups in different times of the day
now   = time.strftime('%H%M%S')
#
target_directory = "/home/utab/"
target = target_directory + today + "_" + now + \
         '.tgz'

# do the actual back up
backup_cmd = "tar -C ~ -zcvf %s %s" % ( target , ' '.join(newer_files) )
status = os.system(backup_cmd)

if status == 0:
    print message_end
else:
    print "Back-up failed"

The script is vulnerable to an injection attack. If anyone can write files under any of the directories to be backed up then the script can be made to execute code of their choosing. (I realize that this is your home directory and "this could never happen" but bear with me for a moment). The problem is that a non-validated string is passed to os.system():

backup_cmd = "tar -C ~ -zcvf %s %s" % ( target , ' '.join(newer_files) )
status = os.system(backup_cmd)

Attacker just needs to:

cd some/where/writable
touch '; rm -rf ~'

But it doesn't even need to be a malicious attack. If any of the files have characters in their names that are interesting to the shell then the script may fail. A tamer example: if any filename contains a hash character (#) then all subsequent filenames in the newer_files list will be ignored and will not be backed up by tar(1).

Instead please consider using os.exec*() or better still the subprocess module. These take a vector of arguments rather than a string and so do not suffer from the same problem.


Multi-threading with TPL - Access Internal Class Properties I am using the TPL library to parallelize a 2D grid operation. I have extracted a simple example from my actual code to illustrate what I am doing. I am getting the desired results I want and my computation times are sped up by the number of processors on my laptop (12).

I would love to get some advice or opinions on my code as far as how my properties are declared. Again, it works as expected, but wonder if the design could be better. Thank you in advance.

My simplified code:

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace Gridding
{

    public abstract class Base
    {
        /// <summary>
        /// Values representing the mesh values where each node in the gridis assigned som value.
        /// </summary>
        public float[,] Values { get; set; }
        public abstract void Compute();
    }

    public class Derived : Base
    {

        /// <summary>
        /// Make the mesh readonly.  Is this necessary?
        /// </summary>
        readonly Mesh MyMesh;

        Derived(Mesh mesh)
        {
            MyMesh = mesh;
        }

        public override void Compute()
        {
            Values = new float[MyMesh.NX, MyMesh.NY];

            double[] xn = MyMesh.GetXNodes();
            double[] yn = MyMesh.GetYNodes();

            /// Paralellize the threads along the columns of the grid using the Task Paralllel Library (TPL).
            Parallel.For(0, MyMesh.NX, i =>
            {
                Run(i, xn, yn);
            });
        }

        private void Run(int i, double[] xn, double[] yn)
        {
            /// Some long operation that parallelizes along the columns of a mesh/grid
            double x = xn[i];
            for (int j = 0; j < MyMesh.NY; j++)
            {

                /// Again, longer operation here
                double y = yn[j];

                double someValue = Math.Sqrt(x * y);
                Values[i, j] = (float)someValue;
            }
        }

        static void Main(string[] args)
        {
            int nx = 100;
            int ny = 120;
            double x0 = 0.0;
            double y0 = 0.0;
            double width = 100;
            double height = 120;

            Mesh mesh = new Mesh(nx, ny, x0, y0, width, height);

            Base tplTest = new Derived(mesh);
            tplTest.Compute();

            float[,] values = tplTest.Values;

            Console.Read();

        }

        /// <summary>
        /// A simple North-South oriented grid.
        /// </summary>
        class Mesh
        {
            public int NX { get; } = 100;
            public int NY { get; set; } = 150;
            public double XOrigin { get; set; } = 0.0;
            public double YOrigin { get; set; } = 0.0;
            public double Width { get; set; } = 100.0;
            public double Height { get; set; } = 150.0;
            public double DX { get; }
            public double DY { get; }

            public Mesh(int nx, int ny, double xOrigin, double yOrigin, double width, double height)
            {
                NX = nx;
                NY = ny;
                XOrigin = xOrigin;
                YOrigin = yOrigin;
                Width = width;
                Height = height;
                DX = Width / (NX - 1);
                DY = Height / (NY - 1);
            }

            public double[] GetYNodes()
            {
                double[] yNodeLocs = new double[NY];
                for (int i = 0; i < NY; i++)
                {
                    yNodeLocs[i] = YOrigin + i * DY;
                }
                return yNodeLocs;
            }

            public double[] GetXNodes()
            {
                double[] xNodeLocs = new double[NX];
                for (int i = 0; i < NX; i++)
                {
                    xNodeLocs[i] = XOrigin + i * DX;
                }
                return xNodeLocs;
            }
        }
    }
}

Values of the Base class should have protected set;.

In Mesh there is inconsistency with properties. Most are not readonly while some are.

Depending of whether or not those properties should be readonly you may remove Values initialization as well as xn and yn computation from Compute.

Furthermore, regardless of previous points, depending on whether or not Compute is called a lot in your application you might want to have a run context with array as a field so that a closure is not created.
And then you can even turn lambda in Parallel.For into a method pointer to a private method.
What this will allow you to do is allocate memory for 2 less object in Compute, which again based on frequency of usage can give you a performance boost.

Why? Firstly, allocation of memory is a slow operation. Secondly, garbage collection freezes all threads while is works. So less objects to collect - better.


Show one form that corresponds to the selected radio button This piece of code I have written works, but I don't think its the best solution.

What I am doing is check if a certain radio is checked if it is show this div if else show this div if this radio is checked etc.

Can someone point me in the right direction on this one? I would love to know how to write less do more.

$(document).ready(function () {
    $('.Form').hide();
    $('input').click(function () {
        <!--check if its One-->
        if ($('input[value=informed]:checked').length) {
            $('#ContactFormOne').show();
            $("#ContactFormTwo, #ContactFormThree, #ContactFormFour, #ContactFormFive, #ContactSix'").hide();
            <!--check if its Two-->
        } else if ($('input[value=release]:checked').length) {
            $('#ContactFormTwo').show();
            $("#ContactFormOne, #ContactFormThree, #ContactFormFour, #ContactFormFive, #ContactSix'").hide();
            <!--check if its Three-->
        } else if ($('input[value=intake]:checked').length) {
            $('#ContactFormThree').show();
            $("#ContactFormTwo, #ContactFormOne, #ContactFormFour, #ContactFormFive, #ContactSix'").hide();
            <!--check if its Four-->
        } else if ($('input[value=checklist]:checked').length) {
            $('#ContactFormFour').show();
            $("#ContactFormTwo, #ContactFormOne, #ContactFormThree, #ContactFormFive, #ContactSix'").hide();
            <!--check if its Fiver-->
        } else if ($('input[value=health]:checked').length) {
            $('#ContactFormFive').show();
            $("#ContactFormTwo, #ContactFormOne, #ContactFormThree, #ContactFormFour, #ContactSix'").hide();
        }
    })
});

Though your conditions are fine, I'd suggest you for a radical change in the code.

Change the ids as follows:

ContactFormOne to informed
ContactFormTwo to release
ContactFormThree to intake
ContactFormFour to checklist
ContactFormFive to health

The code will get a lot shorter now.

The code will be:

$(document).ready(function () {
    $('.Form').hide();
    $('input').on( 'change', function () {
        <!--check if its One-->
        var ValUe = $(this).val();
        $('.Form').hide(); //Replace the '.Form' selector with the element that is containing your contact forms.
        $('#' + ValUe).show();
    })
});

where, I'm assuming you have jQuery 1.9.1.

For a better answer, please include your HTML in the question too.


Improving search through XML file I'm working on a function that extracts information from an XML document and compares to what the user has typed in. Can you suggest any ways in which I can improve it? The variables in the XML file are compared against txtSearch.

function checkXML() {
    var s = "";
    var albumPages = [];

    for (var i = 0; i < xmlalbums.length; i++) {
        var xmlalbum = xmlalbums[i];
        var album_title = dataFromTag(xmlalbum, 'title');
        var artist = dataFromTag(xmlalbum, 'artist');
        var xmltracks = xmlalbum.getElementsByTagName('track');
        for (var t = 0; t < xmltracks.length; t++) {
            var track = xmltracks[t];
            song_title = dataFromTag(track, 'title')
            albumPages[albumPages.length] = artist.toUpperCase() + ':' + album_title.toUpperCase() + ':' + song_title.toUpperCase();
        }
    }

    var resultList = '';
    var resultCount = [];

    for (var i = 0; i < albumPages.length; i++) {
        var searchArtist = albumPages[i].substring(0, albumPages[i].indexOf(":"));
        var searchAlbum = albumPages[i].substring(albumPages[i].indexOf(":") + 1, albumPages[i].lastIndexOf(":"));
        var searchSong = albumPages[i].substring(albumPages[i].lastIndexOf(":") + 1, albumPages[i].length);
        if (searchArtist.indexOf(txtSearch) != -1 || searchAlbum.indexOf(txtSearch) != -1 || searchSong.indexOf(txtSearch) != -1) {
            resultList += "<option>" + searchSong + "</option>"
            resultCount[resultCount.length] = i;
        }
    }

    if (resultList.length == 0) {
        s += "No result found!"
    }
    else if (resultList.length > 0) {
        s += '<select size="' + (resultCount.length + 1) + '" onclick="parent.col3(value);">';
        s += resultList;
        s += '</select>';
    }
    else {
        s += "Awaiting search query..."
    }

    with(document.getElementById('col2').contentDocument) {
        open();
        write(s);
        close();
    }
}

A few general things from a quick scan:

declare all your variables at the top in one var statement rather than using it throughout the script
use somearray.push(x) rather than somearray[somearray.length]=x, as its shorter and means you don't have to look up the .length property every time in your loop
in general, avoid with; there are many reasons why e.g. http://www.yuiblog.com/blog/2006/04/11/with-statement-considered-harmful/
string concatenation is slow; instead of many += operators, create an array, use .push() as above to add strings to the array, then use .join('') to merge them together.

And yes, XPath would be a better approach to extract data from XML.


GildedRose refactoring kata This is GildedRose refactoring kata. Can this code be written to be more readable, and maintainable?
class GildedRose:

    def __init__(self, items):
        self.items = items

can I improve this part of code?
    @staticmethod
    def update_min_max_quality(item):
        """Updates min and max values of items"""
        if item.quality >= 50:
            item.quality = 50
        if item.quality < 0:
            item.quality = 0
        return item.quality

    def decrease_quality(self, item):
        """Decreases item value by 1"""
        self.update_min_max_quality(item)
        item.quality -= 1
        return item.quality

    @staticmethod
    def decrease_sell_in(item):
        """Decreases item sell in by 1"""
        item.sell_in -= 1
        return item.sell_in

    @staticmethod
    def update_aged_brie(item):
        """Updates Aged Brie sell in and quality"""
        if item.sell_in < 0:
            item.quality += 2
        return item.quality

How about this function?
    @staticmethod
    def update_backstage(item):
        """Updates Backstage quality"""
        if item.sell_in <= 0:
            item.quality = 0
            return item.quality
        elif item.sell_in < 6:
            item.quality += 3
            return item.quality
        elif item.sell_in < 11:
            item.quality += 2
            return item.quality
        else:
            return item.quality

    @staticmethod
    def update_conjured(item):
        """Updates conjured items sell in and quality"""
        item.quality -= 2
        item.sell_in -= 1
        return item.quality, item.sell_in

    @staticmethod
    def update_sulfuras(item):
        """Update sulfuras sell in"""
        item.sell_in -= 1
        return item.sell_in

No switches in python, how to shorten this section?
    def update_quality(self):
        """Updates quality and sell in of provided item"""
        for item in self.items:
            if item.name == 'Aged Brie':
                self.update_aged_brie(item)
            if item.name == 'Backstage passes to a TAFKAL80ETC concert':
                self.update_backstage(item)
            if item.name == "Conjured Mana Cake":
                self.update_conjured(item)
            if item.name == "Sulfuras, Hand of Ragnaros":
                self.update_sulfuras(item)
            if item.name in ['+5 Dexterity Vest', "Elixir of the Mongoose"]:
                self.decrease_sell_in(item)
                self.decrease_quality(item)
            if (item.sell_in < 1 or item.quality == 50) and item.name != "Sulfuras, Hand of Ragnaros":
                self.update_min_max_quality(item)

class Item:
    def __init__(self, name, sell_in, quality):
        self.name = name
        self.sell_in = sell_in
        self.quality = quality

    def __repr__(self):
        return "%s, %s, %s" % (self.name, self.sell_in, self.quality)

The individual methods in your code are fine; the structure is what hurts its maintainability. By adding specific update methods to the main Item class, you are giving it a huge amount of responsibility. Adding a layer of abstraction could let you write a simpler Item class and implement the specifics of the item aging process somewhere else.
The trick with this challenge is to recognize the assumptions that are being made by all items, and which parts of the program change for different items. For example, the update logic only ever affects the quality and sell_in properties. Furthermore, these properties change based only on the state of that individual item.
With this in mind, the item class can be written as this, where ager() returns an object with a nextQuality() and a nextSellIn() method, which return the item's values for the next day.
class Item:
    def __init__(self,name,sell_in,quality):
        self.ager = ager(name)

        self.name = name
        self.sell_in = sell_in
        self.quality = quality

    def age(self):
        self.quality = self.ager.nextQuality(self)
        self.sell_in = self.ager.nextSellIn(self)

    def __repr__(self):
        return "%s, %s, %s" % (self.name, self.sell_in, self.quality)

Then, an ager could be written like this, taking in the item as an input.
class DefaultAger:
    @staticmethod
    def nextQuality(item):
        return clamp_quality(item.quality-1)
    @staticmethod
    def nextSellIn(item):
        return item.sell_in-1

Right, we need that clamp_quality(). There's no need to mutate the item with this function, as the mutation already occurs in the age() method of Item. clamper() returns a function that clamps values between lo and hi.
def clamper(lo,hi):
    return lambda val:max(lo,min(hi,val))
clamp_quality = clamper(0,50)

With all of this, we can add a barebones implementaton of ager(). This will handle all items that don't have any special rules.
def ager():
  return DefaultAger

Okay that's great and all, but the entire point of this program is the edge cases. Lets get those in. All of this logic can be handled using more Ager objects.
class ConjuredAger(DefaultAger):
    @staticmethod
    def nextQuality(item):
        return clamp_quality(item.quality-2)

class LegendaryAger(DefaultAger):
    @staticmethod
    def nextQuality(item):
        return item.quality
    @staticmethod
    def nextSellIn(item):
        return item.sell_in

class BackstageAger(DefaultAger):
    @staticmethod
    def nextQuality(item):
        if item.sell_in<=0:
            return 0
        if item.sell_in<5:
            return clamp_quality(item.quality+3)
        if item.sell_in<10:
            return clamp_quality(item.quality+2)
        return clamp_quality(item.quality+1)

Now for the beautiful part. If we dynamically give each item the correct ager, then all we need to do is call item.age() to update an item.
#maps items by name to the correct ager
agers={
    "Aged Brie":BackstageAger,
    "Sulfuras":LegendaryAger,
    "Backstage Pass":BackstageAger,
    "Mystical Hat":ConjuredAger
}
def ager(name):
    if name in agers:
        return agers[name]
    else:
        return DefaultAger

To verify it works:
items = [
    Item("Aged Brie",5,10),
    Item("Sulfuras",5,80),
    Item("Backstage Pass",5,20),
    Item("Jar",20,8)
]
# print this out to make sure we
# set all the agers correctly
print([
    item.ager for item in items
])

for i in range(10):
    print("========= Day %d ========="%i)
    for item in items:
        print(item)
        item.age()

I realize this probably comes off as me showing off my own solution to the problem , but it's the best way I could demonstrate this idea of delegating responsibility into smaller objects. If a new type of item were to be added, one would only need to write an ager and assign it; the Item class could remain untouched.


Equal heights in pure JS I'm pushing myself to learn pure JavaScript, so this morning I wrote an equal heights script in jQuery and then converted it to JavaScript. It works, but I'm using two for loops. Is there a better or more efficient way to write this code, maybe by finding a way to remove the second for loop?

I've created a jsfiddle to make is a little easier to test.

var  maxHeight = -1
,    elems = document.getElementsByClassName("features");

for( var i = 0; i < elems.length; i++ )
{
    maxHeight = maxHeight > elems[i].offsetHeight ? maxHeight : elems[i].offsetHeight;
}

for( var i = 0; i < elems.length; i++ )
{
    elems[i].style.height = maxHeight + "px";
}

The two loops are necessary:
to know the max value, you need to make one complete pass.
There's no way around it.
Only after you know the max value,
after checking all items,
you need a second pass to set the height to the max.

In terms of writing style,
there isn't really a standard,
and it may be a matter of taste,
but in any case I prefer this way:

var maxHeight = -1;
var elems = document.getElementsByClassName("features");

for (var i = 0; i < elems.length; i++) {
    maxHeight = maxHeight > elems[i].offsetHeight ? maxHeight : elems[i].offsetHeight;
}

for (var i = 0; i < elems.length; i++) {
    elems[i].style.height = maxHeight + "px";
}


Exhaustive search for encryption keys This does the job, but is not especially elegant.

What is the preferred Pythonic idiom to my childish nests?

def bytexor(a, b):
    res = ""
    for x, y in zip(a, b):
        if (x == "1" and y == "0") or (y =="1" and x == "0"):
            res += "1"
        else:
            res += "0"

    return res

def get_all():
    res = []
    bit = ["0","1"]
    for i in bit:
        for j in bit:
            for k in bit:
                for l in bit:
                    res.append(i+j+k+l)
    return res

if __name__=="__main__":

    for k0 in get_all():
        for k1 in get_all():
            for k2 in get_all():
                for k3 in get_all():
                    for k4 in get_all():
                        if bytexor(bytexor(k0, k2), k3) == "0011":
                            if bytexor(bytexor(k0, k2), k4) == "1010":
                                if bytexor(bytexor(bytexor(k0,k1),k2),k3) == "0110":
                                    print k0, k1, k2, k3, k4
                                    print bytexor(bytexor(bytexor(k0, k1),k2),k4)

To convert an integer to a bit string with a particular number of digits, use string.format with the b format type. For example:

>>> ['{0:04b}'.format(i) for i in range(16)]
['0000', '0001', '0010', '0011', '0100', '0101', '0110', '0111',
 '1000', '1001', '1010', '1011', '1100', '1101', '1110', '1111']

But really it looks to me as though you don't want to be working with bit strings at all. You just want to be working with integers.

So instead of get_all, just write range(16) and instead of bytexor, write ^. Your entire program can be written like this:

from itertools import product
for a, b, c, d, e in product(range(16), repeat=5):
    if a ^ c ^ d == 3 and a ^ c ^ e == 10 and a ^ b ^ c ^ d == 6:
        print a, b, c, d, e, a ^ b ^ c ^ e

(This will also run a lot faster without all that fiddling about with strings.)


Improve if-statement for multiple strings I working with a lot of Strings which I get from a web-form:

final String user = request.getParameter("user");
final String pw = request.getParameter("password");
final String name = request.getParameter("name");
final String shortname = request.getParameter("shortname");
final String appl_name = request.getParameter("appl_name");
final String db_size_str = request.getParameter("db_size");
final String was_admin_string = request.getParameter("was_admin");
final String sap_nr = request.getParameter("sap_nr");
final String etl_string = request.getParameter("etl");
final String mq_string = request.getParameter("mq");
final String msg_broker_string = request.getParameter("msgbroker");
final String java_batch = request.getParameter("javab");
final String server = request.getParameter("server");
final String apverantw1 = request.getParameter("appliverantw1");
final String apverantw2 = request.getParameter("appliverantw2");
final String psp = request.getParameter("psp");
final String version = request.getParameter("version");
final String applialias = request.getParameter("applialias");
final String applityp = request.getParameter("applityp");
final String data = request.getParameter("data");
final String yesdata = request.getParameter("yesdata");
final String moreoptions = request.getParameter("moreoptions");

If only one of them is empty, the program should be stopped. At this time I check that with an ugly if-statement if no one of the strings is empty:

if (user != null && name != null && shortname != null && appl_name != null && db_size_str != null && was_admin_string != null && sap_nr != null && etl_string != null && mq_string != null
                && msg_broker_string != null && java_batch != null && server != null && apverantw1 != null && apverantw2 != null && psp != null && version != null && applialias != null
                && applityp != null && data != null && yesdata != null && moreoptions != null)

This was produced over the time when I added one string after one.
How can I improve this? I mean, it is a really big, ugly if-statement. But I don't have a clue how can I write this even better.

i assume that you don't want to proceed the request, and send error message about missing string  to the user, and also you are not using any MVC framework.

First you need to define a list of parameter names.  Then define the below method,

 List<String> paramNames =  ... // you prepared a list of parameter names

List<String> validate(HttpServletRequest req ) {
   List<String> emptyParams = new ArrayList<>();

   for(String name : paramNames ){
      if(StringUtils.isBlank( req.getParameter(name))){
          emptyParams.add(param);
       }
   }

   return emptyParams;
 }

Note: StringUtils class comes from the apache common library.

now the above method can tell you what parameters are empty. you can decide what to do based on the result.

if you use any mvc framework,  it is better to use form binding logic and validation provided by the framework.


Stack Overflow tab notifier user-script I searched for a notification script that shows inbox and rep change items on the browser tab in stackapps.com but either they didn't work or were outdated.

So, I've done this raw script myself and want to make it fool proof and robust:
This is my first ever user script. So want to make it better. (Even if it wasn't the first, I would have wanted to make it better)

// ==UserScript==
// @name         Tab Notifier
// @namespace    http://2200ce.wordpress.com/
// @version      0.1
// @description  A notification system that displays inbox and rep change notification on the tab
// @author       Amit Joki - http://stackoverflow.com/users/3001736/amit-joki
// @include      http://stackoverflow.com/*
// @grant        none
// @require      https://cdn.rawgit.com/kapetan/jquery-observe/master/jquery-observe.js
// @match        *://*.stackexchange.com/*
// @match        *://*.stackoverflow.com/*
// @match        *://*.superuser.com/*
// @match        *://*.serverfault.com/*
// @match        *://*.askubuntu.com/*
// @match        *://*.stackapps.com/*
// @match        *://*.mathoverflow.net/*
// ==/UserScript==

var userscript = function($) {
    $('.icon-inbox .unread-count, .icon-achievements .unread-count').observe({
        attributes: true,
        attributeFilter: ['style']
    }, function() {
        var title = document.title;
        var isInbox  = $(this).parent().is(".icon-inbox") ? true : false;
        if (!$(this).attr('style')) {
            document.title = updater(title, $(this), isInbox);
        } else if ($(this).attr("style") == "display: none;") {
            debugger;
            var frags =  title.replace(/• /,"•").split(/\s(?=\w)/);
            var rgx = (isInbox ? /\[\d+\]/ : /\[\+\d+\]/);
            if(frags[0].indexOf("•") == -1){
                title = frags[0].replace(rgx, "") + frags.slice(1).join(" ");
            } else {
                rgx = /^(\[)(\+\d+) •(\d+)(\])/;
                title = frags[0].replace(rgx, isInbox ? "$1$2$$4" : "$1$3$4");
            }
            document.title = title + frags.slice(1).join(" ");
        } else { }

    });

    function updater(title, elm, isInbox){
        var txt = elm.text().trim();
        var rgx = (isInbox ? /\[\d+\]/ : /\[\+\d+\]/);
        if(title.indexOf("•") > -1){
            rgx = isInbox ? /^(\• )\d+/ : /^(\[)+\d+/;
            title = title.replace(rgx, "$1" + txt);
        }
        else if(rgx.test(title)){
            title = title.replace(rgx, "[" + txt + "] ");
        }
            else {title = "[" + txt + "] " + title;}

        if(title.indexOf("[")> -1 && title.match(/\[/g).length == 2){
            var arr = title.split(/\s(?=\w)/), rtn =['[',,' • ',,']'];
            arr[0].split(" ").forEach(function(e){
                rtn[e.indexOf("+") > -1 ? 1 : 3] = e.replace(/\[|\]/g,"");
            });
            title = rtn.join("") + " " + arr.slice(1).join(" ");
        }
        return title;
    }
}
var el = document.createElement('script');
el.type = 'text/javascript';
el.text = '(' + userscript + ')(jQuery);';
document.head.appendChild(el);

What this does is when inbox item alone comes up, it shows as:

  [{number of inbox item here}]

and same for rep change:

  [+{rep change here}]

When both come, they will be displayed in the following style:

  [+{rep change here} • {inbox items here}]

Also once the notifications are seen, it will be subsequently removed as well. I'm using MutationsObservers, which is warped by a plugin called jQuery.observe which looks to settle cross-browser issues.

How can this be improved?

Instead of this:

    var isInbox  = $(this).parent().is(".icon-inbox") ? true : false;

Since .is(...) is boolean, you can use its returned value directly without the ternary operator:

    var isInbox  = $(this).parent().is(".icon-inbox");

This else is pointless, just delete it:

    } else { }

The indentation is off in the else block, and the formatting doesn't follow common conventions:

    else if(rgx.test(title)){
        title = title.replace(rgx, "[" + txt + "] ");
    }
        else {title = "[" + txt + "] " + title;}

It would be better to write this code like this:

    else if(rgx.test(title)){
        title = title.replace(rgx, "[" + txt + "] ");
    } else {
        title = "[" + txt + "] " + title;
    }

In this code:

var rgx = (isInbox ? /\[\d+\]/ : /\[\+\d+\]/);
if (title.indexOf("•") > -1) {
    rgx = isInbox ? /^(\• )\d+/ : /^(\[)+\d+/;
    title = title.replace(rgx, "$1" + txt);
}
// ...

It's misleading that rgx is first assigned to something,
then it's reassigned to something else inside the if branch.
This makes the reader look suspiciously at the code after the if statement to see if rgx is used again for something.

Looking at the code, rgx is not used again.
Which means,
you're just reusing the variable inside the if branch.
But it's not a good practice to reuse a variable for more than one purpose.
It would be better to create a new variable inside the if branch,
for example:

var rgx = (isInbox ? /\[\d+\]/ : /\[\+\d+\]/);
if (title.indexOf("•") > -1) {
    var rgx2 = isInbox ? /^(\• )\d+/ : /^(\[)+\d+/;
    title = title.replace(rgx2, "$1" + txt);
}


Actively writing an HTTPS webserver in Python 3 Please help me improving this existing, working code. I am not asking you to change what it does, as it handles connections already perfectly as designed by my own specs. I just want the code to be reviewed to be sure.

#!/usr/bin/env python3
# -*- coding: utf-8 -*-
import os, ssl
from socketserver import ThreadingMixIn
from http.server import SimpleHTTPRequestHandler, HTTPServer

global sslcontext
sslcontext = ssl.create_default_context(purpose=ssl.Purpose.CLIENT_AUTH)
sslcontext.options |= ssl.OP_NO_TLSv1
sslcontext.options |= ssl.OP_NO_TLSv1_1
sslcontext.protocol = ssl.PROTOCOL_TLSv1_2
sslcontext.set_ciphers("ECDHE-ECDSA-AES256-GCM-SHA384 ECDHE-ECDSA-CHACHA20-POLY1305")
sslcontext.set_ecdh_curve("secp384r1")
sslcontext.load_cert_chain("/home/pi/keys/fullchain.pem", "/home/pi/keys/privkey.pem")

class HSTSHandler(SimpleHTTPRequestHandler):
    def end_headers(self):
        self.send_header("Strict-Transport-Security", "max-age=63072000; includeSubDomains; preload")
        SimpleHTTPRequestHandler.end_headers(self)

class ThreadedHTTPServer(ThreadingMixIn, HTTPServer):
    daemon_threads = True

def main():
    try:
        os.chdir("/media/external")#auto-change working directory
        SimpleHTTPRequestHandler.sys_version = "0.1"#display custom Python system version
        SimpleHTTPRequestHandler.server_version = "0.2"#display custom server software version
        my_server = ThreadedHTTPServer(('', 443), HSTSHandler)
        my_server.socket = sslcontext.wrap_socket(my_server.socket, server_side=True)
        print('Starting server, use <Ctrl-C> to stop')
        my_server.serve_forever()

    except KeyboardInterrupt:
        print(' received, shutting down server')
        my_server.shutdown()

if __name__ == '__main__':
    main()

When importing names from a location (module, package, dynamic library, ...), don't mix multiple locations on a single line. That is, this:

import os, ssl

... should be:

import os
import ssl

This is purely a stylistic concern, and it's one of the rules listed in PEP-8, the official Python style guide.
Global variables should be a last resort. They clutter the global namespace, are hard to track when reading the code, and are very prone to race conditions when using threads. Here, you could pass the context as an argument, or perhaps set up another class with the context attached as an attribute.

Let's say you have to share foo between func_a() and func_b(). Three different approaches:

Make foo global (analogous to what you did with the context, not great):

foo = Foo()

def func_a():
    foo.method_1()

def func_b():
    foo.method_2()

func_a()
func_b()

Pass foo as an argument:

def func_a(foo):
    foo.method_1()

def func_b(foo):
    foo.method_2()

def main():
    foo = Foo()
    func_a(foo=foo)
    func_b(foo=foo)

Make foo a class attribute:

class MyClass:
    def __init__(self, foo):
        self.foo = foo

    def func_a(self):
        self.foo.method_1()

    def func_b(self):
        self.foo.method_2()

my_class = MyClass(Foo())
my_class.func_a()
my_class.func_b()

Even if you stick to using global variables, setting global sslcontext is unnecessary because you're not reassigning it. If you're not assigning to a variable, Python will:

i. Try to find the name locally;

ii. If that fails, look for the name in global scope;

iii. Finally, raise a NameError to indicate the variable doesn't exist in any scope.

Therefore, if you reference sslcontext locally, the interpreter will find the global name (ii.). But it can't read your mind. If you instead assign sslcontext = 42, Python will assume you want to create a fresh, local variable with the name 'sslcontext' and the value of 42, rather than reassigning the global sslcontext. That's it. That's where global comes in, and that's all it's good for.
In HSTSHandler.end_headers(), this idiom:

SimpleHTTPRequestHandler.end_headers(self)

... could be more clearly expressed by calling super(), then super().end_headers(). This is good practice, and also assures multi-inheritance is handled correctly.
Source code encoding does not need to be set explicitly here, as ASCII will suffice.

That's as much as I can give you without more context to the question.


Banner Picker in jQuery I would like to know if my current jQuery code could be improved? If yes, can you give me improvements?

$(document).ready(function($)
{
    $("#bannerPickerToggleIcon").click(function(e)
    {
        e.preventDefault();
        $("#bannerPicker").slideToggle();
        $("this").css({"display": "block"});
        $("#bannerPickerToggleIcon").toggleClass("active");
    });
    $("#bannerPicker span").click(function()
    {
        var bannerid = $(this).attr("id");
        $.cookie('bannerimg', bannerid,
        {
            expires: 365,
            path: '/'
        });
        $("body").removeClass().addClass(bannerid);
    });
    if (($.cookie('bannerimg') !== null))
    {
        $("body").addClass($.cookie('bannerimg'));
    }
    else
    {
        $("body").addClass("bannerDefaultImage");
    }
});
</script>

The code allows to open a selection of banners allowing the user to select a banner from the selection. Once a user has selected a banner, this one is saved via a cookie to save the selection. I created a cookie js file. It's the default cookie file from jQuery. Currently the code works, but I would like to know if it could be improved.

You don't need to wrap your JQuery code in $(document).ready(function() { ... }). See this Stackoverflow question here for more details on that.

The general style for brackets is that the open bracket is on the same line of the declaration, like this below example.

function f() {
    ...
}

Other than that, there's not much else here that I can see that's glaringly wrong, or in need of review. If there's anything else that you want me to cover, just ask, and I'll see what I can do.


Implementation of a Fraction class in Java I am writing a bunch of Math related classes:BigInteger,Fraction,matrix,vector,polynomial,set etc.All of these classes will use each other.The classes will either work in decimal format or in Rational format at a time(eg as FractionalMatrix and DecimalMatrix) so I thought it will be best to implement the Fraction class first.

My primary concern is that this is going to be a big project.So, code readability and management is very important.Any suggestions on making the code better are appreciated.

Here is the Fraction class:

public final class Fraction {

private int numerator;

private int denominator;

public Fraction(int numerator,int denominator,boolean wantToReduce) {
    if (denominator == 0) {
        throw new IllegalArgumentException("The denominator is zero.");
    }
    if(numerator==0){
        this.numerator = 0;
        this.denominator = 1;
    }
    else{
    this.numerator = numerator;
    this.denominator = denominator;
    }
    if(denominator<0){
        this.numerator = -1*this.numerator;
        this.denominator = -1*this.denominator;
    }
    if(wantToReduce==true)
        this.reduce();
}
public Fraction(int num) {
    this.numerator = num;
    this.denominator = 1;
}

@Override
public String toString() {
    if(denominator!=1)
    return numerator+"/"+denominator;
    else
        return numerator+"";
}

@Override
public boolean equals(Object obj) {
    if(!(obj instanceof Fraction))
        return false;
    Fraction f = ((Fraction)obj);
    int gcd= Math2.gcd(numerator,denominator);
    f.reduce();
    if(this.numerator/gcd==f.numerator && this.denominator/gcd == f.denominator)
        return true;
    else
        return false;
}

public Fraction reduce(){
    int gcd = Math2.gcd(numerator,denominator);
    numerator = numerator/gcd;
    denominator = denominator/gcd;
    return this;
}

//Cannot decide weather to make static methods or not so I randomly picked one.
public static Fraction add(Fraction f1,Fraction f2,boolean w){
    return new Fraction(f1.numerator*f2.denominator+f2.numerator*f1.denominator,f1.denominator*f2.denominator,w);
}
public static Fraction sub(Fraction f1,Fraction f2,boolean w){
    return new Fraction(f1.numerator*f2.denominator-f2.numerator*f1.denominator,f1.denominator*f2.denominator,w);
}
public static Fraction mul(Fraction f1,Fraction f2,boolean w){
    return new Fraction(f1.numerator*f2.numerator,f1.denominator*f2.denominator,w);
}
public static Fraction div(Fraction f1,Fraction f2,boolean w){
    return new Fraction(f1.numerator*f2.denominator,f1.denominator*f2.numerator,w);
}

}

Here is the Math2.gcd method, I have used The Euclidean Algorithm for finding the GCD.

 public static int gcd(int a,int b){
    if(a<0)
        a=-1*a;
    if(b<0)
        b=-1*b;
    int t;
    while(b!=0){
        t=b;
        b = a%b;
        a = t;
    }
    return a;
}

I'm not going to repeat the basics which Ronan has covered fairly comprehensively. I'd just add: why use -1*a when you could use -a?

However, I have some design questions:

Reduction

I have a Rational class which I have used for about 15 years. It always reduces, and I've never found this to be a problem. Why would you want unreduced fractions?

In addition, consider that being reduced or not is not really an property of the number: it's a property of how the number is displayed in text. If you have use cases, would they be covered equally well or better by a toString method which takes a desired denominator?

Among the advantages of using reduced numbers are i) it simplifies equality testing; ii) it reduces the risk of overflow (or improves memory usage and speed, if using BigIntegers for numerator and denominator).

Mutability

Whether you keep unreduced numbers or not, make them immutable. In fact, I would recommend making every class in your CAS immutable, and using the builder pattern if you think this makes it too hard to instantiate matrices.

int vs BigInteger

Why are the numerator and denominator stored as ints? They will easily overflow, especially given that adding two rationals requires multiplying numerators and denominators. If you do stick with ints to keep memory usage low then you should cast to longs in the arithmetic methods and then reduce before casting back to ints, but I would recommend using BigInteger.

A final note

Well done for considering the special cases around zero and negative numbers in the constructor.


Getting translated string using reflection and Resource Manager I wrote the following code to get the translated string from Resource Manager. The method GetTranslatedString is called more than 50 times in my project.

  public static string GetTranslatedString(string input, string culture)
    {
        if (string.IsNullOrEmpty(input)) return input;
        string retInput = input;
        CultureInfo cultureInfo = new CultureInfo(culture);
        ResourceManager rm = new ResourceManager("Globalization.XYZ", Assembly.GetExecutingAssembly());

        var entry = rm.GetResourceSet(cultureInfo, true, true)
            .OfType<DictionaryEntry>()
            .FirstOrDefault(e => e.Key.ToString().Equals(input, StringComparison.InvariantCultureIgnoreCase));
        if (entry.Value == null) return retInput;
        return entry.Value.ToString();

    }

But I feel it's not perfect code because the ResourceManager class gets new instances many times and since I'm calling GetTranslatedString many times I am sure it will impact on performance.

Second major problem is, I am using reflection inside this code. and it's a costly operation.

It would helpful if I could get review on this.

public static string GetTranslatedString(string input, string culture)
{
    if (string.IsNullOrEmpty(input)) return input;
    string retInput = input;
    CultureInfo cultureInfo = new CultureInfo(culture);
    ResourceManager rm = new ResourceManager("Tesla.TSS.Globalization.SchedulingSystem", Assembly.GetExecutingAssembly());

    var entry = rm.GetResourceSet(cultureInfo, true, true)
        .OfType<DictionaryEntry>()
        .FirstOrDefault(e => e.Key.ToString().Equals(input, StringComparison.InvariantCultureIgnoreCase));
    if (entry.Value == null) return retInput;
    return entry.Value.ToString();

}

Because the resources of your application won't change during runtime, the easiest way would be to once get the whole items and store them in a dictionary. Then your application can use the dictionary to retrieve the translated strings.

I would like to suggest to make this a little bit more flexible, so you could use it in other projects too.

At first the method which will retrieve the Dictionary<string,string> from the ResourceManager

public static Dictionary<string, string> GetTranslations(CultureInfo cultureInfo, string baseName, Assembly assembly)
{
    ResourceManager rm = new ResourceManager(baseName, assembly);

    using (ResourceSet rs = rm.GetResourceSet(cultureInfo, true, true))
    {
        return rs.OfType<DictionaryEntry>()
                 .ToDictionary(r => r.Key.ToString(),
                      r => r.Value.ToString());
    }
}

as you can see, this can be called independent from your current project.

Now let us add an overloaded method which takes the culture as a string like so

public static Dictionary<string, string> GetTranslations(string culture, string baseName, Assembly assembly)
{
    CultureInfo cultureInfo = new CultureInfo(culture);
    return GetTranslations(cultureInfo, baseName, assembly);
}

In your project you can call it now like

Dictionary<string,string> translations = GetTranslations(culture, "Tesla.TSS.Globalization.SchedulingSystem", Assembly.GetExecutingAssembly());

and you could query this to get the translated strings.

You could have a method ChangeLanguage(string culture) like so

private string currentCultureName = "en-Us";
private void ChangeLanguage(string culture)
{
    if (string.IsNullOrWhiteSpace(culture))
    {
        currentCultureName = CultureInfo.CurrentUICulture.Name;
    }
    else
    {
        currentCultureName = culture;
    }

    translations = GetTranslations(culture, "Tesla.TSS.Globalization.SchedulingSystem", Assembly.GetExecutingAssembly());
}

to change the current dictionary.

Your former GetTranslatedString() method should then live as a class method (not static) inside your project and should look like so

public string GetTranslatedString(string input, string culture)
{

    if (input == null) { throw new ArgumentNullException("input"); }

    if (string.IsNullOrWhiteSpace(input)) { return input; }

    if (currentCultureName != culture)
    {
        ChangeLanguage(culture);
    }

    string translation;
    if (translations.TryGetValue(input, out translation))
    {
        return translation;
    }

    return input;
}

As you can see I have change quite a few things like

adding braces {} although they are optional for single line if statements, but they will make the code less error prone.
for a passed in input which is null this will throw an ArgumentNullException which is much better than just returning the null.
for the passed in culture I didn't add a null check, because if the ChangeLanguage() method will be called with a null value the name of the CurrentUICulture will be used.


Questionnaire classes and interfaces I Need your help, please. I'm trying to develop a simple Questionnaire.

Here is what I did already. First, I have interfaces for Question and Answer entities.

interface IAnswerProvider{

    public function getAnswers();

}

interface IQuestionProvider{
    public function getQuestions();
}

Then I have classes which implement these interfaces. Both Answers and Questions have translations in different languages - that's why each of them have a $current_language property.

class Answer implements IAnswerProvider
{

    private $current_language = '';

    private $current_question_id = 0;

    public function __construct($current_language, $question_id = 0)
    {
        if (!empty($question_id)) {
            $this->setCurrentQuestionId($question_id);
        }

        $this->setCurrentLanguage($current_language);
    }

    public function getAnswers()
    {

        $question_id = $this->getCurrentQuestionId();

        if (empty($question_id)) {
            throw new Error('Question ID is not defined');
        }

        $query = 'select * from  answers a   where a.question_id=' . escape($question_id);

        return DB::getInstance()->fetchAllRows($query);
    }

    /**
     * @return string
     */
    public function getCurrentLanguage()
    {
        return $this->current_language;
    }

    /**
     * @param string $current_language
     */
    public function setCurrentLanguage($current_language)
    {
        $this->current_language = $current_language;
    }

    /**
     * @return int
     */
    public function getCurrentQuestionId()
    {
        return $this->current_question_id;
    }

    /**
     * @param int $current_question_id
     */
    public function setCurrentQuestionId($current_question_id)
    {
        $this->current_question_id = $current_question_id;
    }

}

class Question implements IQuestionProvider
{

    private $current_language = '';

    public function __construct($current_language)
    {
        $this->setCurrentLanguage($current_language);
    }

    /**
     * @return string
     */
    public function getCurrentLanguage()
    {
        return $this->current_language;
    }

    /**
     * @param string $current_languge
     */
    public function setCurrentLanguage($current_language)
    {
        $this->current_language = $current_language;
    }

    public function getQuestions()
    {

        $query = 'select * from questions q where language ='.escape($this->getCurrentLanguage());

        return DB::getInstance()->fetchAllRows($query);
    }

}

And here is a class which does all jobs related with questions and answers.

class Questionnaire{

    private $oQuestionProvider = '';

    private $oAnswerProvider = '';

    public function __construct(IQuestionProvider $oQueston, IAnswerProvider $oAnswer)
    {
        $this->oQuestionProvider = $oQueston;
        $this->oAnswerProvider = $oAnswer;
    }

    /**
    * Returns an array of questions , each question has answers property
    * with array of answers
    */
    public function getQuestionsWithAnswers(){

        $aData = $this->oQuestionProvider->getQuestions();

        //assign answers to each question
        foreach ($aData as $v){
            $this->oAnswerProvider->setCurrentQuestionId($v->question_id);
            $v->answers = $this->oAnswerProvider->getAnswers();
        }

        return $aData;
    }

}

What I 'm concerned about is that I have code duplication in the Question and Answer classes - I mean setCurrentLanguage and getCurrentLanguage methods. Maybe it's better to move them into some abstract class and then inherit both Answer and Question  classes from it ? Are SOLID principles being violated here?

Please let me know what do you think about this code.
Thanks

This is not a real answer, but it is too long to be a comment.

I would be very lazy, in this case, and define the language at a global level. You could even make it a constant, because it probably won't change during the running of the script. If you want to keep language setters and getters in every object then I would indeed use a parent class for that.

I also wonder if you reconnect to the database for every query? That's very inefficient. I hope you reuse the connection. Also, even though the variables are not under user control, I would, as a habit, use parameter binding in queries, for security reasons. Better use it too often than too little.

Also try to use sensible names for your variable and field names. The use of 'current' in some, and not in others, seems weird. Remember, they are always 'current' when used. Names have to make sense. Just remove the word 'current' everywhere and it's fixed.


Countdown clock in Python This project is from the subreddit for beginner Python projects:

  The goal of the project is to create a program that allows the user to choose a time and date, and then prints out a message at given intervals (such as, every 3 seconds) that tells the user how much longer there is until the selected time. If the chosen time has passed, the program lets the user know. When selecting the month, the user has the option of entering the month name or number.

Is there a simple and succinct way to remove the microseconds from being printed out? And is there a better way to set up the while loop?

import datetime
import time

# a simple program that allows the user to pick a time and the program counts down
# until that time has been reached

month_strings = ['jan','feb','mar','apr','may','jun','jul','aug','sep','oct','nov','dec',]

# This function creates a datetime object chosen by the user and compares
# this object to the current time and counts down until that chosen time
def count_down():

    year = int(raw_input('What is the year of the date? '))
    month = raw_input('What is the month of the date? ')
    # this part allows the user to type in the month as a string or integer
    if len(month) < 3:
        month = int(month)
    else:
        month = (month_strings.index(month[0:3].lower()) + 1)
    day = int(raw_input('What is the day of the month? '))
    hour = int(raw_input('What is the hour according to a 24 hour clock? '))
    minute = int(raw_input('At which minute? '))
    second = int(raw_input('At which second? '))

    chosen_time = datetime.datetime(year,month,day,hour,minute,second)

    print 'The time you chose is: ' + chosen_time.__str__()

    current_time = chosen_time.now()
    # If the time has passed, let the user know and exit the program
    if chosen_time < chosen_time.now():
        print 'This time has passed.'
        exit()

    else:
        # This loop prints how much time is left until the chosen time every 3 seconds
        while True:
            time_difference = (chosen_time - chosen_time.now())
            print str(time_difference) + ' until the time.'
            time.sleep(3)
            if chosen_time <= chosen_time.now():
                break
        print "It's time!"
        exit()

if __name__ == '__main__':
    count_down()

I'll first just suggest that you read the Python style guide as it has great tips on readability.

The best way you can format time is using datetime.strftime. You can call it on a datetime object and pass a string formatted to represent each time value and it will format the data into the string format you provide. This way you can format an exact code you'd like instead of just removing milliseconds:

print (datetime.datetime.now().strftime("%Y-%m-%d %H:%M:%S"))
>>> 2015-09-17 15:43:30

Using the table here you can see all the available options if you'd like to go more specific. While I'm on the datetime, it is not clear when you call .now on your stored object. It reads awkwardly. Instead use datetime.datetime.now(). But, since that's a lot to type I'd instead suggest calling from datetime import datetime at the start, so you can then just call datetime.now().

On your second question, yes there is a better way to do the while loop. Instead of using if to check conditions and break on them, make that condition the condition of your while loop.

while chosen_time >= datetime.now():
    time_difference = (chosen_time - chosen_time.now())
    print str(time_difference) + ' until the time.'
    time.sleep(3)

The reason people usually use while True is that either the condition is not easy to make into an expression, the condition can't be evaluated when the loop is firt encountered or that they want to emulate a do while loop. But none of these is true for you, so it's not needed.

Other Notes

You almost never need to call __str__. The point of functions with two underscores on either side is that they allow on object to be passed to another function and run that way. In this case, you could use str(chosen_time). But I wouldn't use that either, instead use str.format.

print 'The time you chose is: {}'.format(chosen_time)

It replaces the {} with the parameter you pass into format. It's very useful when you have multiple parameters or need to apply particular formatting to them, so you should get used to using it.

You don't need brackets around the `time_difference calculation, so leave them out.

time_difference = chosen_time - chosen_time.now()

I agree with Ethan about the input validation, but I want to specifically note that someone could enter "12 ". That would seem too long to be an integer, but would actually require that you call int on it, which you currently don't. I suggest that you call strip on the month parameter. It will remove whitespace at the start and end of the user's input, preventing this issue.

month = raw_input('What is the month of the date? ').strip()


Header file for Bezier curve utility This is a header file for my bezier curve utility and several of BezierCurve2D's methods return newly created arrays of Point2D objects.

Should I create (using new) and delete objects in the same scopes? Should I have these methods receive a pre-allocated space of Point2Ds?

#ifndef BEZIER_UTILITY_H
#define BEZIER_UTILITY_H

class Point2D {
public:
    double X;
    double Y;
    Point2D();
    Point2D(double X, double Y);
    void set(double X_p, double Y_p);
    void set(Point2D* point_p);
};

class BezierBase {
protected:
    int values_length;
    double* temp_space;
    int temp_space_bytes;
    double* interp (double t, double* values);
public:
    BezierBase(int values_length);
    ~BezierBase();
};

class BezierCurve2D: public BezierBase {
private:
    double* Xs;
    double* Ys;
    double calc_dist (double x1, double x2, double y1, double y2);
    Point2D* linear_raster (double* values, int max_results, double goal_dist, double tolerance);
public:
    BezierCurve2D(double* x_values, double* y_values, int values_length);
    Point2D* rasterize (int max_results, double goal_dist, double tolerance);
    Point2D* rasterizeToX (int max_results, double goal_dist, double tolerance);
    Point2D* rasterizeToY (int max_results, double goal_dist, double tolerance);
    Point2D* getT (double t);
    double measure (double segment_dist, double tolerance);
};

#endif

Raw memory management should be incredibly rare and a last resort.

Your best bet will be some kind of container that can manage the memory for you. Returning a pointer to an allocated chunk of memory has all kinds of problems (who owns it and thus should delete[] it? what if you forget to release it? what about exception safety? etc).

In your situation, std::vector is almost certainly the right choice. It has essentially the same performance as a raw array if it's used wisely, and it gives pleasant, safe value semantics to an otherwise dangerous dynamic allocation.

In addition to the standard containers, it can also be valuable to know about std::shared_ptr and std::unique_ptr1. They give ownership semantics to memory allocations and facilitate safe handling of dynamically allocated memory. More plainly, they allow for the automatic releasing (e.g. delete or delete[]ing) of a memory allocation when the smart pointer (or all of the smart pointers in the case of std::shared_ptr) goes out of scope scope.

1 If you cannot use C++11, there are also some Boost smart pointers that do not require C++11.


Clicking child view and preventing parent view from running dblclick event I have a parent view which has a dblclick event handler bound to it. I also have a view which is a descendant of the parent. This child view has a click event handler bound to it.

Currently, my code works as intended, but looks like this:

var ParentView = Backbone.Marionette.ItemView.extend({
    events: {
        'dblclick': '_playInStream'
    },

    _playInStream: function () {
        console.log("ParentView _playInStream");
    }
});

var ChildView = Backbone.Marionette.ItemView.extend({
    events: {
        'click': '_addToStream',
        'dblclick': '_addToStream'
    },

    _addToStream: _.debounce(function () {
        console.log("ChildView _addToStream");

        //  Don't allow dblclick to bubble up to the list item and cause a play.
        return false;
    }, 100, true)
});

The end result of this logic is:

Single-click on child view runs _addToStream (once)
Double-click on child view runs _addToStream (once)
Double-click on parent view runs _playInStream (once)

However, I feel like the way I am expressing this code is overly convoluted, but I also don't see a better way.

The debounce is necessary because double-clicking on ChildView will execute both the click and dblclick event handlers -- but I only want a single event to run.

If I remove the dblclick event handler from the child then a dblclick will propagate up to the parent which is not desireable.

Does anyone see a better way to achieve what I want?

I'm not sure what seems convoluted to you. It's a simple piece of code, and if you remove anything from it, it will fail to do what you want, as you noted yourself:

If you remove the debounce, the method will be fired 3 times: once for the single click + twice for the double-click. So you need that.
If you remove the double click handler, then it will be fired in the parent

There is one thing you could "optimize", in the ChildView you could make the dblclick handler do nothing:

var ChildView = Backbone.Marionette.ItemView.extend({
    events: {
        'click': '_addToStream',
        'dblclick': function() {}
    },

This will work, because the dblclick will also trigger click, so _addToStream will be called. You still cannot remove the debounce, because without it the dblclick will trigger 2 click events.

Finally, the ChildView doesn't extend the ParentView, maybe that's a typo, and your real intention was this:

var ChildView = ParentView.extend({