Effective ways to find partial sha1 collision I need to find 2 different string and compare their hash value. Both string must contain "abc". I looking for the first eight character that are the same and stop. I have been running for more than 24 hours and still not found. My code has no problem running but i am wondering is there any more effective ways to find the collision

public static void main(String[] args) {
String message1 = "abc";
String message2 = "abc";

int x=0;
if (message1.equals(message2)) {
    String temp1 = message1 + x;
    String temp2 = x + message2;
    String result1 = sha1Hashing(temp1);
    String result2 = sha1Hashing(temp2);
    while (!result1.equals(result2)){
        temp1 = message1 + x;
        temp2 = x + message2;
        result1 = sha1Hashing(temp1);
        result2 = sha1Hashing(temp2);
        System.out.println("First message = " + temp1 + " Second message = " + temp2 + "\n");
        System.out.println("First hash = " + result1);
        System.out.println("Second hash = " + result2 + "\n");
        x++;
        if(result1.equals(result2)){
            System.out.println("FOUND!!");
            System.exit(0);
        }
    }
}
}
public static String sha1Hashing (String message) {
String sha1 = "";
StringBuffer sb = new StringBuffer();
try {
    MessageDigest mDigest = MessageDigest.getInstance("SHA1");
    byte[] result = mDigest.digest(message.getBytes());

    for (int i = 0; i < result.length; i++) {
        sb.append(Integer.toString((result[i] & 0xff) + 0x100, 16).substring(1));
    }
} catch(NoSuchAlgorithmException e) {
    e.printStackTrace();
}
//return first 8 char
sha1 = sb.toString().substring(0,8);
return sha1;
}

Choosing two strings, hashing them, and comparing the first 8 hex characters of the hash should produce a match once every 2^32 iterations.  The question implies that the values for x for the match aren't required to be the same, so your approach is much too restrictive.  You need to compare the hash you just generated with all of the previous hashes.  If you use a Dictionary<String, String> or similar to store previous results, you can expect a collision after 2^16 elements.

Coding-wise, your byte-to-string conversion can be made more efficient (you do work that is discarded).  You might consider just returning 8 bytes and writing a comparitor for that.


Reuse the value of previous type instead of None I've created a function in python to print some information through it. I'm trying to supply list of alphabets to the function along with the type those alphabets fall under.

When I execute the function, It checks whether the value of the type is None. If it is None, it executes the except block to fetch one.

I've tried with (working one):

def get_info(alpha,alpha_type):

    print("checking value of item:",alpha_type)

    try:
        if not alpha_type:raise
        return alpha,alpha_type
    except Exception:
        alpha_type = "vowel"
        return get_info(alpha,alpha_type)

if __name__ == '__main__':
    main_type = None
    for elem in ["a","e","o"]:
        result = get_info(elem, alpha_type=main_type)
        main_type = result[1]
        print(result)

Output it produces (as expected):

checking value of item: None
checking value of item: vowel
('a', 'vowel')
checking value of item: vowel
('e', 'vowel')
checking value of item: vowel
('o', 'vowel')

Although the value of the type is None when an alphabet is supplied to the function, ain't there already the value of previous type (when run twice) stored in the memory. However,
When I try like the following, I get messy output:

def get_info(alpha,alpha_type):

    print("checking value of item:",main_type)

    try:
        if not alpha_type:raise
        return alpha,alpha_type
    except Exception:
        alpha_type = "vowel"
        return get_info(alpha,alpha_type)

if __name__ == '__main__':
    for elem in ["a","e","o"]:
        print(get_info(elem,alpha_type=None))

Output the second function produces:

checking value of item: None
checking value of item: vowel
('a', 'vowel')
checking value of item: None
checking value of item: vowel
('e', 'vowel')
checking value of item: None
checking value of item: vowel
('o', 'vowel')

I would like to get the expected output (what the first function produces) following the design of the second function, so that I don't need to influence the result from outside of the function.

First, your code reformatted properly so it's more legible:

def get_info(alpha, alpha_type):

    print("checking value of item:", alpha_type)

    try:
        if not alpha_type:
            raise

        return alpha, alpha_type

    except Exception:
        alpha_type = "vowel"
        return get_info(alpha, alpha_type)

if __name__ == '__main__':
    main_type = None
    for elem in ["a", "e", "o"]:
        result = get_info(elem, alpha_type=main_type)
        main_type = result[1]
        print(result)

Take 20 minutes and read over PEP8. It's Python's style guide and dictates how code should look. Most notably, you should have spaces after commas, and you shouldn't try to shove things onto one line in most cases (like you're doing with raise).

Your code is abusing try and raise. It doesn't make any sense to raise in a try just to go to the except. You should just use make use of the if that you already have:

def get_info(alpha, alpha_type):

    print("checking value of item:", alpha_type)

    if not alpha_type:
        return get_info(alpha, "vowel")

    else:
        return alpha, alpha_type

Honestly, I don't understand what you're trying to do here, so that's really all I can comment on. I just wanted to point out the formatting and needless use of try.


Implement a job scheduler which takes in a function f and an integer n, and calls f after n milliseconds public static void main(String args[]) {
        solution(new Command() {
            @Override
            public void execute() {
                {
                    System.out.println("Hello World");
                }
            }
        }, 1000);

        // java 8
        solution(() -> System.out.println("Hello World"), 1000);
    }

    public static void solution(Command command, int n) {
        new java.util.Timer().schedule(new java.util.TimerTask() {
            @Override
            public void run() {
                command.execute();
            }
        }, n);

    }

    interface Command {
        public void execute();
    }

}

Can my solution be improved? I have used command pattern. Is there a better way to do this?

Imports

Use import java.util.Timer; and import java.util.TimerTask; at the top of the file.  This will allow you to simplify this:

new java.util.Timer().schedule(new java.util.TimerTask() {

to this:

new Timer().schedule(new TimerTask() {

which requires less typing and less reading, and so is arguably easier to understand.

Naming

command is a decent name for a Command argument; n is a terrible name for an int.  Is that the count of the number of command objects you want submitted?  You have to consult the documentation for Timer#schedule() to discover that it is the number of milliseconds to wait before executing the command.

milliseconds, or delay_ms would be better names.

Similarly, solution() is a terrible name.  Solution to what?  schedule_command() might be a better choice.

Reuse existing interfaces

The difference between interface Command and interface Runnable is that the former uses execute() where as the latter uses run().  Oh, and the latter also adds the marker @FunctionalInterface.

By creating your own Command interface, you would force anyone who wants to use your scheduler, who already has classes that implement the standard Runnable interface, to add another interface to their class and forward the execute() method to the run() method (or vis versa).

Timers and Threads

Every Command passed to solution() will cause the creation of a new Timer() object, with its own timer thread.  Each Timer object can schedule multiple tasks.  Additionally, all tasks owned by a Timer execute sequentially in the same thread, reducing concurrency issues.  Consider:

for (int i=0; i<3; i++) {
    solution( () -> { System.out.print("Hello from command #");
                      Thread.sleep(1);
                      System.out.print(i);
                      Thread.sleep(1);
                      System.out.println("!"); }, 1000 );
}

When executed, the output might become:

  Hello from command #Hello from command #0Hello from command #!
  12!
  !

If your scheduler used a single, common Timer object ...

private static Timer timer = new Timer();

public static void solution(Command command, int n) {
    timer.schedule(new TimerTask() {
        @Override
        public void run() {
            command.execute();
        }
    }, n);
}

Then you would be guaranteed this output instead:

  Hello from command #0!
  Hello from command #1!
  Hello from command #2!

even though each Command was scheduled to run at the same time.

Program Termination

From the Timer documentation:

  After the last live reference to a Timer object goes away and all outstanding tasks have completed execution, the timer's task execution thread terminates gracefully (and becomes subject to garbage collection). However, this can take arbitrarily long to occur.

If you want your program to terminate quickly, you need to call .cancel() on all Timer objects you create, once the last scheduled task has completed (or at least been started).  This could be done by scheduling a task to cancel the task timer:

timer.schedule( () -> timer.cancel() );

Job Control

You can .cancel() a TimerTask before it has begun executing.  However, nothing is returned from schedule(), so the caller cannot cancel any submitted tasks.

Better Implementation

Again, from the Timer documentation:

  Java 5.0 introduced the java.util.concurrent package and one of the concurrency utilities therein is the ScheduledThreadPoolExecutor ...

private static ScheduledExecutorService executor;

public static void main(String args[]) {
    executor = Executors.newSingleThreadScheduledExecutor();

    ScheduledFuture<?> future = solution( () -> System.out.println("Hello World"), 1000);

    // Use future here, if desired.

    // Cleanup
    executor.shutdown();
    executor = null;
}

public static ScheduledFuture<?> solution(Runnable command, int delay_ms) {
    return executor.schedule( command, delay_ms, TimeUnit.MILLISECONDS );
}


Fractional knapsack in C++11 I am just picking up C++11 and decided to try some algorithmic problems to get better at it. One such problem is the fractional knapsack problem which can be solved by a greedy approach. The idea is to find the most valuable item and fill it in and move on to the next one till the knapsack is filled. Partial items are allowed.

I hope this code can be reviewed for:

Correctness
Correct usage of C++11 features
Code logic improvement
Coding conventions

struct Item {
    float value;
    float size;
    uint32_t index;
};

uint32_t FillKnapsack(std::vector<Item> &Items, const uint32_t MaxCap)
{
    // sorting is done based on value/size ratio of the item
    std::sort(Items.begin(),
              Items.end(),
              [](const Item& A, const Item& B) { return (A.value/A.size) > (B.value/B.size); });

    float c = 0, v = 0;

    for (const auto& item: Items) {
        if (c + item.size <= MaxCap) {
            c += item.size;
            v += item.value;
        }
        else {
            v += (MaxCap - c)/item.size * item.value;
            c = MaxCap;
        }

        std::cout << "adding item " << item.index << " with value " << item.value << std::endl;
        std::cout << "c: " << c << " v: " << v << std::endl;

        // since the knapsack is filled we can break out.
        if (c >= MaxCap)
            break;
    }
    return v;
}

int main()
{
    std::vector<Item> items = { { 10, 30, 0},
                                { 20, 20, 1},
                                { 30, 10, 2} };
    std::cout << FillKnapsack(items, 35) << std::endl;
    return 0;
}

Return or Output

I feel like FillKnapsack should either (a) simply tell you the max value or (b) give you the items that it used. Right now, you're returning (a) and logging, incompletely, (b). (Incomplete because if you use a fractional amount of an item, that's not indicated).

I suggest you pick one or the other. If the former, basically just drop the couts. If the latter, you'll want to return something like a:

std::vector<std::pair<Item, double>>

or:

struct ItemWithFraction {
    Item item;
    double fraction;
};
std::vector<ItemWithFraction>

Avoid single-letter variables

What is c? What is v?

Move the break into the branch

We have two cases per item: either we can fit the whole item and have room to spare, or we can't. If we can't, then we break. So that can become:

for (const auto& item : Items) {
    if (capacity + item.size < MaxCap) {
        value += item.value;
        capacity += item.size;
    }
    else {
        // last little bit left
        value += (MaxCap - capacity) / item.size * item.value;
        break;
    }
}
return value;

Don't need to assign capacity in the else case either.

Take items by copy

The idea of the algorithm suggests that given this set of items, find me a value. It doesn't suggest to me that you should reorder the items I pass in just because that happens to make it easier to solve the problem. To avoid being destructive, I'd write the signature as:

uint32_t FillKnapsack(std::vector<Item> Items, const uint32_t MaxCap)

If the user doesn't care, they can always move() the items in and avoid the copy.

Naming

Typically, UpperCase naming are used for classes (e.g. Item). Functions and variables are typically either camelCase (with the first letter lower) or snake_case. So I'd prefer

fillKnapsack(std::vector<Item> items, const uint32_t maxCapacity);

and

fill_knapsack(std::vector<Item> items, const uint32_t max_capacity);

to what you have.


It'th wabbit theathon I don't know regex. I want to learn regex, but learning regex just for the sake of regex would be quite boring.

So I decided to learn Ruby as well (which so far looks a lot like Python, which I'm familiar with).

My first program attempts to speak with a combination of both Daffy Duck's lisp and Elmer Fudd's verbal apraxia.

It works quite well, but it looks quite messy already. How does one properly combine Ruby and (repeated) regex substitutions?

def lisp(input)
   return input.downcase.gsub(/s/, "th").gsub(/er/, "uh").gsub(/r|l/, "w").capitalize
end

print lisp("What's your input?")
user_input = gets.chomp

puts lisp(user_input)

I downcase everything to make the regex easier and capitalize at the end to turn it back into a proper sentence.

Usage:

What'th youw input? It's rabbit season!

  It'th wabbit theathon!

What'th youw input? My super soaker carries 2 liters of water!

  My thupuh thoakuh cawwieth 2 wituhth of watuh!

In general, it is bad practice to perform string substitutions in multiple passes, feeding the output back of one substitution into another round of substitutions.  (Here is an example of how multiple-pass substitutions can lead to incorrect results.)  In this case, it happens to be safe, since none of the outputs overlap with any of the patterns, but I would still recommend a different approach.  Fortunately, Ruby makes it easy to do multiple substitutions the right way, with #gsub(pattern, hash).

return is implicit in Ruby.  The last expression evaluated is automatically returned, and everything is an expression.

The standard is to use two spaces of indentation, not three.

def lisp(input)
  input.downcase.gsub(/er|[lrs]/,
    'er' => 'uh',
    'l'  => 'w',
    'r'  => 'w',
    's'  => 'th',
  ).capitalize
end


Managing rental charges for movies The condition for refactoring is:

Define 2 new rules with respect to rental charges that are applicable and with the ability to add new categories of movies.
Ex: Rental amount for children's movie for more than 2 days should be 100 from 3rd day onwards.

I have 3 classes (Children, Movie and Rental). I need suggestions on code smell and refactoring it.
Customer.java
import java.util.ArrayList;
import java.util.Iterator;

public class Customer {

    private String name;
    private ArrayList rentals = new ArrayList();

    public Customer(String name) {
        this.name = name;
    }

    public void addRental(Rental arg) {
        rentals.add(arg);
    }

    public String getName() {
        return name;
    }

     public String statement() {

        double totalAmount = 0;
        int frequentRenterPoints = 0;

        String returnVal = "Rental record for : " + this.getName() + ".....\n";

        Iterator iter = rentals.iterator();

        while(iter.hasNext()) {
            double thisAmount = 0;
            Rental each = (Rental) iter.next();
            //determine amounts for each line
            switch (each.getMovie().getPriceCode()) {
                case Movie.REGULAR: thisAmount += 100;
                if (each.getDaysRented() > 2)
                    thisAmount += (each.getDaysRented() - 2) * 75;
                break;

                case Movie.NEW_RELEASE:
                thisAmount += each.getDaysRented() * 150;
                break;

                case Movie.CHILDREN:
                thisAmount += 75;
                if (each.getDaysRented() > 3)
                thisAmount += (each.getDaysRented() - 3) * 75;
                break;
            }

            // add frequent renter points
            frequentRenterPoints ++;
            // add bonus for a two day new release rental
            if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && each.getDaysRented() > 1)
                frequentRenterPoints ++;

            //show figures for this rental
            returnVal += "\t" + each.getMovie().getTitle() + ": " + thisAmount + "\n";
            totalAmount += thisAmount;

        }

        //print footer
        returnVal += "Amount owed is: " + totalAmount + "\n";
        returnVal += "You have earned " + frequentRenterPoints + " frequent renter points";

        return returnVal;
    }
}

Movie.java
public class Movie {

    public static final int CHILDREN = 2;
    public static final int REGULAR = 0;
    public static final int NEW_RELEASE = 1;

    private String title;
    private int priceCode;

    public Movie(String title, int priceCode) {
        this.title = title;
        this.priceCode = priceCode;
    }

    public int getPriceCode() {
        return priceCode;
    }

    public void setPriceCode(int arg) {
        priceCode = arg;
    }

    public String getTitle() {
        return title;
    }
}

Rental.java
class Rental {

    private Movie movie;
    private int daysRented;

    public Rental(Movie movie, int daysRented) {
        this.movie = movie;
        this.daysRented = daysRented;
    }

    public int getDaysRented() {
        return daysRented;
    }

    public Movie getMovie() {
        return movie;
    }
}

The first thing I noticed about your code, is that you don't clearly have a movie category. I did find these methods on Movie after a while

public int getPriceCode() {
    return priceCode;
}

public void setPriceCode(int arg) {
    priceCode = arg;
}

I believe the getter would better be renamed to getCategory(). I would also completely remove the setter. It's really unlikely for a movie to change categories, so it should only be set during construction.

It might also be worth writing a specific class Category to list all possible categories. An enum would be the most natural here, but this might be awkward with the requirement:

  with the ability to add new categories of movies.

If it's no problem to recompile the code with a new movie (as I expect is the case in this example program) then the enum is still the better choice here.

public enum Category {
    CHILDREN, REGULAR, NEW_RELEASE;
}

This also changes the Movie class to this:

public class Movie {
    private final String title;
    private final Category category;

    public Movie(String title, Category category) {
        this.title = title;
        this.category = category;
    }

    public Category getCategory() {
        return category;
    }

    public String getTitle() {
        return title;
    }
}

Notice that I also added final to the fields to make it clear that a movie instance is immutable. Immutability is nice to have.

The other thing I didn't expect in your code is that the Customer class is responsible for calculating the "statement".

If I wanted to add a new category to the code, I wouldn't expect that I need to change the Customer class as well.

There are 2 major ways to solve this smell. Either put the "statement" calculations into a specialised class. Or add category specific calculations to that category.

Let's take a look into the second option since this might not be trivial if you're not used to working with enums in java.

I would implement it like this:

public enum Category {
    CHILDREN {
        @Override
        public int calculateRent(int daysRented) {
            int result = 75;
            if (daysRented > 3)
                result += (daysRented - 3) * 75;
            return result;
        }
    }, REGULAR {
        @Override
        public int calculateRent(int daysRented) {
            int result = 100;
            if (daysRented > 2)
                result += (daysRented - 2) * 75;
            return result;
        }
    }, NEW_RELEASE {
        @Override
        public int calculateRent(int daysRented) {
            return daysRented*150;
        }
    };

    public abstract int calculateRent(int daysRented);
}

This way, we can simplify the statement() in Customer to:

public String statement() {
    double totalAmount = 0;
    int frequentRenterPoints = 0;

    String returnVal = "Rental record for : " + this.getName() + ".....\n";

    for(Rental rental : rentals){
        double thisAmount = rental.getMovie().getCategory().calculateRent(rental.getDaysRented());

        frequentRenterPoints ++;
        if ((rental.getMovie().getCategory() == Category.NEW_RELEASE) && rental.getDaysRented() > 1) {
            frequentRenterPoints ++;
        }

        returnVal += "\t" + rental.getMovie().getTitle() + ": " + thisAmount + "\n";
        totalAmount += thisAmount;

    }

    returnVal += "Amount owed is: " + totalAmount + "\n";
    returnVal += "You have earned " + frequentRenterPoints + " frequent renter points";

    return returnVal;
}

With this change, adding a new Category only changes a single class (the enum).

Some other things to point out that I improved in this method:

foreach

Since java 5 I believe, there's a specialised way to loop over all itmes in a collection. The syntax here is for(Rental rental : rentals) which reads as "for each rental in rentals".

meaningful names

I renamed your each variable to rental. If you read this later on in the method it's more clear that it's the rental you're talking about.

comments are clutter

Comments are rarely useful. A comment should never be added to say what some line of code does. The only exception being a javadoc for methods (because you usually look at the javadoc from outside the class if you're interested in what that method does that it's name doesn't already tell you).

If you really need a comment to be able to understand some line(s) of code, you should either rename variables/methods in that line or extract a method with a name that clearly says what happens.

Always put {}

You should put curly brackets after each for/if/else/...
This is to prevent really difficult to debug mistakes like this:

if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && each.getDaysRented() > 1)
            System.out.println("Added bonus frequenterpoints");
            frequentRenterPoints ++;

All I did was add a print to see if anyone had bonus points added. But this innocent looking change cause EVERYONE to get a bonus point independent of the if check.

Most IDE's can be configured to automatically add the braces for you.

edit: making rentalpoints dependant on category as well.

Since the ammount of rental points depends on the category too, let's add another method in that enum. Since it only changes for a new release, I would add a default implementation that returns 1 and override this in the NEW_RELEASE. The new Category enum now looks like this:

public enum Category {
    CHILDREN {
        @Override
        public int calculateRent(int daysRented) {
            int result = 75;
            if (daysRented > 3)
                result += (daysRented - 3) * 75;
            return result;
        }
    }, REGULAR {
        @Override
        public int calculateRent(int daysRented) {
            int result = 100;
            if (daysRented > 2)
                result += (daysRented - 2) * 75;
            return result;
        }
    }, NEW_RELEASE {
        @Override
        public int calculateRent(int daysRented) {
            return daysRented*150;
        }

        @Override
        public int calculateFrequentRenterPoints(int daysRented) {
            if(daysRented > 1){
                return 2;
            } else {
                return 1;
            }
        }
    };

    public abstract int calculateRent(int daysRented);
    public int calculateFrequentRenterPoints(int daysRented){
        return 1;
    }
}

Now there's one more principle that I too have to get used to. It's the tell, don't ask. This is violated inside the statement method.

What we're doing is asking a rental for it's movie, then asking the movie for it's category and then once more asking the rental for the number of days rented. And then do the same asking for the frequent renter points.

This is too much asking. It would make more sense to just tell rental to calculate the price and then return the result. This way we only ask for one thing.

If we move those 2 methods to the Rental class istead, then our statement method will turn to this:

public String statement() {
    double totalAmount = 0;
    int frequentRenterPoints = 0;

    String returnVal = "Rental record for : " + this.getName() + ".....\n";

    for(Rental rental : rentals){
        frequentRenterPoints += rental.calculateFrequentMoviePoints();

        double thisAmount = rental.calculateRent();
        returnVal += "\t" + rental.getMovie().getTitle() + ": " + thisAmount + "\n";
        totalAmount += thisAmount;
    }

    returnVal += "Amount owed is: " + totalAmount + "\n";
    returnVal += "You have earned " + frequentRenterPoints + " frequent renter points";

    return returnVal;
}

For completeness sake here's the new Rental class as well:

public class Rental {
    private Movie movie;
    private int daysRented;

    public Rental(Movie movie, int daysRented) {
        this.movie = movie;
        this.daysRented = daysRented;
    }

    public int getDaysRented() {
        return daysRented;
    }

    public Movie getMovie() {
        return movie;
    }

    public int calculateRent() {
        return movie.getCategory().calculateRent(daysRented);
    }

    public int calculateFrequentMoviePoints() {
        return movie.getCategory().calculateFrequentRenterPoints(daysRented);
    }
}

This actually makes a lot more sense. A Customer only knows that it can ask a rental for the price and points, it doesn't need to know how those are calculated.

A Rental needs to know who to ask for the actual price or points, and has the information needed to calculate those (in this case, the number of days rented).

A Category contains category - specific calculations. Adding a new category means only implementing a new field in the enum. No other classes need to be changed.

Your idea of putting it into subclasses of movie (eliminating the Category class altogether) would work too. It's even better for the Rental class since it doesn't need to know that there exist such a thing as different categories of movies.

The only reason I would not go for this solution is the reason to change a Movie class. If your name is just an identifier for the movie, and no other info about the movie is stored in that class than I would completely agree with this being a good solution. Except that I would change the name to something else. (If only I could find a decent name for that specific concept ... naming things is hard). Let's give it the not so awesome name RentableMovie for now.

That would mean a Customer has a bunch of Rentals.
Each of those rentals has a number of days and a RentableMovie.
That RentableMovie contains specific ways of calculating the price and points. It only contains 1 identifier for the movie. Or we could introduce a new MovieInformation class so that the RentableMovie can point to all information about the movie, without knowing what exactly is in it.

This would work as well :)
So you can choose what feels best to you.


Simple C++ thread pool I wrote a minimalistic thread pool using some C++11 features. I've used it on two projects so far and it has worked fine, but when I ran the code on my laptop, I believe that my thread pool might be entering some kind of dead-locked state -- occasionally the CPU activity drops to almost nothing, and the program hangs.

What are your opinions on the code?

My header file:

#include <utils/Uncopyable.h>
#include <vector>
#include <queue>
#include <thread>
#include <mutex>
#include <atomic>
#include <condition_variable>
#include <functional>

/// \brief Use this class to run tasks in parallel.
class ThreadPool : private Uncopyable {
public:
    ThreadPool();
    ThreadPool( size_t threads );
    ~ThreadPool();

    /// \brief Initialize the ThreadPool with a number of threads.
    /// This method does nothing if the thread pool is already running,
    /// i.e. ThreadPool( size_t ) was called.
    void initializeWithThreads( size_t threads );

    /// \brief Schedule a task to be executed by a thread immediately.
    void schedule( const std::function<void()>& );

    /// \brief a blocking function that waits until the threads have processed all the tasks in the queue.
    void wait() const;

private:
    std::vector<std::thread>            _workers;
    std::queue<std::function<void()>>   _taskQueue;
    std::atomic_uint                    _taskCount;
    std::mutex                          _mutex;
    std::condition_variable             _condition;
    std::atomic_bool                    _stop;
};

And my implementation is as follows:

#include <utils/ThreadPool.h>
#include <chrono>

ThreadPool::ThreadPool()
    :   _workers(),
        _taskQueue(),
        _taskCount( 0u ),
        _mutex(),
        _condition(),
        _stop( false ) {}

ThreadPool::ThreadPool( size_t threads ) : ThreadPool() {
    initializeWithThreads( threads );
}

ThreadPool::~ThreadPool() {
    _stop = true;
    _condition.notify_all();
    for ( std::thread& w: _workers ) {
        w.join();
    }
}

void ThreadPool::initializeWithThreads( size_t threads ) {
    for ( size_t i = 0; i < threads; i++ ) {
        //each thread executes this lambda
        _workers.emplace_back( [this]() -> void {
            while (true) {
                std::function<void()> task;
                {   //acquire lock
                    std::unique_lock<std::mutex> lock( _mutex );
                    _condition.wait( lock, [this]() -> bool {
                        return !_taskQueue.empty() || _stop;
                    });

                    if ( _stop && _taskQueue.empty() ) {
                        return;
                    }

                    task = std::move( _taskQueue.front() );
                    _taskQueue.pop();
                }   //release lock
                task();
                _taskCount--;
            }   //while
        });
    }   //for
}

void ThreadPool::schedule( const std::function<void()>& task ) {
    {
        std::unique_lock<std::mutex> lock( _mutex );
        _taskQueue.push( task );
    }
    _taskCount++;
    _condition.notify_one();
}

void ThreadPool::wait() const {
    while ( _taskCount != 0u ) {
        std::this_thread::sleep_for( std::chrono::microseconds(1) );
    }
}

A few minor points, mostly related to coding style and practices:

The non-copyable pattern is somewhat outdated since C++11 deleteable methods. Some might also argue that explicitly deleting the copy constructor and assignment operator is more clear too. Refer to this SO question.
Names starting with an underscore are somewhat contrived in the C++ language. For class members, this naming convention is not ilegal, but you might want to consider other options, such as the m_ prefix or an underscore at the end instead. Refer to this SO question for more on the subject.
I would name wait() as waitAll() or similar, to make it explicit that it will wait for all current tasks to complete.
In the implementation of ThreadPool::wait(), you should probably be using std::this_thread::yield instead of sleep_for.
You could go for an auto & in this loop:

   for ( std::thread& w: _workers )

to facilitate maintenance if you ever change the contents of _workers by some other type.
For absolute consistency, size_t is a member of namespace std, so it should be used as std::size_t.


Processor for handling generic commands I'm trying to implement business logic layer based on concepts of commands and command handlers.

A command is a thing that contains input parameters for executing some action, and it knows what kind of output that action should produce. A command handler contains logic for actually executing an action: it accepts a command as a param, handles it in some way, and (if successful) produces an output object.

public interface ICommand<TResultData>
{ }

public interface ICommandHandler<TCommand, TResultData>
    where TCommand : ICommand<TResultData>
{
    CommandResult<TResultData> Handle(TCommand command);
}

public static class CommandProcessor
{
    public static CommandResult<TResultData> Process<TCommand, TResultData>(TCommand command)
        where TCommand : ICommand<TResultData>
    {
        var handler = ServiceLocator.GetInstance<ICommandHandler<TCommand, TResultData>>();
        return handler.Handle(command);
    }
}
public class CommandResult<TResultData>
{
    public bool Success { get; set; }

    public string Error { get; set; }

    public TResultData Data { get; set; }
}

Example usage:

// CreateUserCommand & handler implementations *very* simplified
public class CreateUserCommand : ICommand<User>
{
    public string Email { get; set; }
}

public class CreateUserCommandHandler : ICommandHandler<CreateUserCommand, User>
{
    private readonly IRepository repository;
    public CreateUserCommandHandler (IRepository repository)
    {
        this.repository = repository;
    }
    public CommandResult<User> Handle(CreateUserCommand command)
    {
        if(this.repository.Users.Any(u=>u.Email == command.Email)
          return new CommandResult<User> { Success = false, Error = "Email already taken"};
        var user = new User {Email = command.Email};
        this.repository.Users.Add(user);
        return new CommandResult<User> { Success = true, Data = user};
    }
}

// command usage in application code
var command = new CreateUserCommand { Email = "some.email.com" };
CommandResult<User> result = CommandProcessor.Process(command);
if(result.Success)
{
    // at this point we know that result.Data is of type User, which is nice
    // so we can use this strictly-typed result data in any way
    User createdUser = result.Data;
    Console.Writeline("Created user with Id = " + createdUser.Id);
}
else
{
    Console.Writeline("Error creating user: " + result.Error);
}

Everything works pretty nice, as you can see in the usage example, but one thing that bothers me is the empty interface ICommand. Is it a bad thing here? Can the code be refactored in some way to make it better?

Your marker interface has no added value. In fact, it's an anti-pattern because you provide a generic type parameter on an interface ICommand, only to enforce it on another interface ICommandResult.

public interface ICommand<TResultData>
{ }

public interface ICommandHandler<TCommand, TResultData>
    where TCommand : ICommand<TResultData>
{
    CommandResult<TResultData> Handle(TCommand command);
}

Since specific commands have nothing in common, I would not create an interface for commands. Also, TResultData has nothing to do with a command, and everything with the command result.

The handler could be rewritten as follows:

public interface ICommandHandler<TCommand, TResultData>
{
    CommandResult<TResultData> Handle(TCommand command);
}


Pythonic way of checking for dict keys When checking a python dict for a key, which is the better way here:
(I can't figure how to use dict.get() to make this cleaner)

if 'y' in self.axes:
    ax = self.axes['y'].ax
    coord1[ax] = x0
    coord2[ax] = (y1) - height[ax]
    coord3[ax] = x0 + width[ax]
    coord4[ax] = y1

or:

try:
    ax = self.axes['y'].ax
except KeyError:
    pass
else:
        coord1[ax] = x0
        coord2[ax] = (y1) - height[ax]
        coord3[ax] = x0 + width[ax]
        coord4[ax] = y1

The latter way is closer to EAFP but former seems clearer, especially for new code contributors who are scientists first & coders second.

Both ways are valid and have their own advantages. Checking upfront using an if statement involve a bit of overheard for each usage, using EAFP involve greater overheads but only for the "wrong" case.

So if you mostly have cases where 'y' is in self.axes then EAFP is better, otherwise LBYL is fine.

But if you only are calling this pattern a handful of times, then either will do; or no, use the third approach contextlib.suppress because this except: pass is ugly:

from contextlib import suppress

with suppress(KeyError):
    ax = self.axes['y'].ax
    coord1[ax] = x0
    coord2[ax] = (y1) - height[ax]
    coord3[ax] = x0 + width[ax]
    coord4[ax] = y1


Consumer-Producer Problem: POSIX Thread I have implemented a producer consumer problem,
following the resources below:

Oracle doc
CSEE

I have used mutex_t and sem_t.  The reason for using mutex_t instead of binary semaphore because of my understanding- mentioned here.

And hence I have avoided using sem_t as binary.

Also the Oracle doc link I have provided uses two mutexes, sem_t & cmut, whereas I have used only one mutex_t object as suggested in the CSEE link provided.

Is it OK?  Please review my code and provide your valuable comments.

#include <stdio.h>
#include <pthread.h>
#include <semaphore.h>

#define MAX 10

typedef struct{
    char buff[MAX];
    int in;
    int out;
    int count;
    pthread_mutex_t mutex;
    sem_t empty;
    sem_t full;
}BUFFER;

BUFFER queue;
char substring[5];

void initializeBuffer(BUFFER* b)
{
    b->count = 0;
    b->in    = 0;
    b->out   = 0;

    pthread_mutex_init(&(b->mutex),NULL);
    sem_init(&(b->full),0,0);
    sem_init(&(b->empty),0,MAX);
}

void destroyBuffer(BUFFER* b)
{
    b->count = 0;
    b->in    = 0;
    b->out   = 0;

    pthread_mutex_destroy(&(b->mutex));
    sem_destroy(&(b->full));
    sem_destroy(&(b->empty));
}

void producer(BUFFER* b,char c)
{
    sem_wait(&(b->empty));
    pthread_mutex_lock(&(b->mutex));

    b->buff[b->in]=c;
    (b->in)++;
    (b->in) %= MAX;
    (b->count)++;

    pthread_mutex_unlock(&(b->mutex));
    sem_post(&(b->full));
}

char consumer(BUFFER* b)
{
    sem_wait(&(b->full));
    pthread_mutex_lock(&(b->mutex));

    char val=(b->buff[b->out]);
    (b->out)++;
    (b->out) %= MAX;
    (b->count)--;

    pthread_mutex_lock(&(b->mutex));
    sem_post(&(b->empty));
    return val;
}

void* THREAD_PRODUCER(void* arg)
{
    char array[]="HelloWorld";
    int i=0;
    printf("Producer Thread Created\n");
    while(array[i] != '\0')
    {
        producer(&queue,array[i]);
        i++;
    }
    pthread_exit((void*) 1);
}

void* THREAD_CONSUMER(void* arg)
{
    int i=0;
    printf("Consumer Thread Created\n");
    for(i=0;i<5;i++)
    {
        substring[i]=consumer(&queue);
    }
    pthread_exit((void*) 1);
}

int main()
{
    pthread_t T1,T2;
    initializeBuffer(&queue);
    pthread_create(&T1,NULL,THREAD_PRODUCER,NULL);
    pthread_create(&T2,NULL,THREAD_CONSUMER,NULL);
    pthread_join(T1,NULL);
    pthread_join(T2,NULL);
    printf("Inside queue: %c\n",queue.buff[queue.out]);
    printf("substring: %s\n",substring);
    destroyBuffer(&queue);
    return 0;
}

The output is:

$ ./ProducerConsumer.exe
Producer Thread Created
Consumer Thread Created
Inside queue: W
substring: Hello

1. Bugs

It doesn't work! On OS X 10.9.4, with Clang/LLVM, when I run it, I get the output:

Producer Thread Created
Consumer Thread Created

... and then it hangs. Here's an LLDB session:

(lldb) run
Process 29359 launched: './a.out' (x86_64)
Producer Thread Created
Consumer Thread Created
^C
(lldb) thread backtrace all
* thread #1: tid = 0x5461f, 0x00007fff8752ca3a libsystem_kernel.dylib`__semwait_signal + 10, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
  * frame #0: 0x00007fff8752ca3a libsystem_kernel.dylib`__semwait_signal + 10
    frame #1: 0x00007fff862c17f3 libsystem_pthread.dylib`pthread_join + 433
    frame #2: 0x0000000100000e17 a.out`main + 135 at cr55792.c:102

  thread #2: tid = 0x54646, 0x00007fff8752c746 libsystem_kernel.dylib`__psynch_mutexwait + 10
    frame #0: 0x00007fff8752c746 libsystem_kernel.dylib`__psynch_mutexwait + 10
    frame #1: 0x00007fff862c0779 libsystem_pthread.dylib`_pthread_mutex_lock + 372
    frame #2: 0x0000000100000c5b a.out`consumer(b=0x0000000100001070) + 155 at cr55792.c:66
    frame #3: 0x0000000100000d57 a.out`THREAD_CONSUMER(arg=0x0000000000000000) + 71 at cr55792.c:90
    frame #4: 0x00007fff862bd899 libsystem_pthread.dylib`_pthread_body + 138
    frame #5: 0x00007fff862bd72a libsystem_pthread.dylib`_pthread_start + 137

Thread #1 is the main thread. It is waiting at line 102 to join the consumer thread:

pthread_join(T2,NULL);

Thread #2 is the consumer. It is waiting to take the lock in the function consumer at line 66:

pthread_mutex_lock(&(b->mutex));

The producer thread has finished.

Obviously the mistake is at line 66 of consumer: it should say pthread_mutex_unlock. The question for you is, how did you miss this? This is not some mysterious race condition: it ought not to be possible for consumer to take the lock twice in succession.
With bug #1 fixed, the program runs to completion. Sometimes it produces the correct output:

Producer Thread Created
Consumer Thread Created
Inside queue: W
substring: Hello

but about half the time it produces the output:

Consumer Thread Created
Producer Thread Created
Inside queue: W
substring:

Why is this? It's because this call in consumer is not blocking when it should:

sem_wait(&(b->full));

Let's see why, by changing this to:

if (sem_wait(&(b->full)) != 0) {
    perror("sem_wait");
    exit(EXIT_FAILURE);
}

Sure enough, this check fails with:

sem_wait: Bad file descriptor

Reading the manual page for sem_wait:

  [EINVAL]           sem is not a valid semaphore descriptor.

Why is that? Let's add an error check to the sem_init call:

if (sem_init(&(b->full),0,0) != 0) {
    perror("sem_init");
    exit(EXIT_FAILURE);
}

This check fails with:

sem_init: Function not implemented

This is because OS X does not support unnamed semaphores (sem_init, sem_destroy). On this operating system, it's necessary to use named semaphores (sem_open, sem_close, sem_unlink) instead (that is, if you need to restrict yourself to POSIX; if you're writing OS X-only code then you could use the Mach semaphores).

The lesson here is that writing portable Unix programs is hard! I wouldn't necessarily suggest that you replace your unnamed semaphores with named semaphores (although that's what I've done in my revised code below), just that you check the result of sem_init and other functions so that portability problems like this are easily discovered and diagnosed.

2. Review

You don't check error codes! Read the manual pages for sem_wait, pthread_mutex_lock and so on and you'll see that they can all fail. Checking error codes would have immediately identified the cause of problem #2 above.

Checking for errors is vital in multi-threaded code, where race conditions are often hard to reproduce. I was lucky that this particular error occurred about 50% of the time: had it been rare I would probably have missed it altogther.

Always check error codes! It's burdensome to start with, but not technically difficult, to develop a simple error-reporting system, and apply it to all fallible function calls. And once in a while this will save your ass.
There are no comments. What do these functions do? What is the meaning of each variable?
Some of your names are lower case (queue, producer), some are uppercase (BUFFER, THREAD_PRODUCER), and some are mixed (initializeBuffer), but there doesn't seem to be any consistent meaning in this distinction. It's worth paying careful attention to names.
You use names inconsistently. For example, is it a BUFFER or a queue?
The names T1 and T2 are rather obscure. One is the producer thread and the other the consumer thread, but which is which? It is hard to remember.
In the BUFFER structure, the in and out members are non-negative indexes into buff, and the count member is a non-negative count of unconsumed items, so they should be represented by unsigned integers, for example size_t.
The count member is not actually used for anything. Use it or remove it! I would suggest using it by adding an assertion before you decrement it.
You have unnecessary parentheses in expressions like:

&(b->empty)
(b->in)++;

which suggests that you are not confident about operator precedence and associativity. I would suggest having an operator precedence table open in a tab somewhere until you get used to it, rather than adding parentheses when you are unsure.

You'll see from the table that -> has higher precedence than &, so it's safe to write:

&b->empty

You'll also see that -> and (postfix) ++ have the same precedence but associate from left to right, so it's safe to write:

b->in++;

There's no need to call pthread_exit at the end of the thread function. If you read the manual, you'll see that:

  An implicit call to pthread_exit() is made when a thread other than the thread in which main() was first invoked returns from the start routine that was used to create it. The function's return value serves as the thread's exit status.

3. Revised code

#include <assert.h>
#include <pthread.h>
#include <semaphore.h>
#include <stdio.h>
#include <stdlib.h>

/* If expr is false, print error message and exit. */
#define CHECK(expr, msg)                        \
    do {                                        \
        if (!(expr)) {                          \
            perror(msg);                        \
            exit(EXIT_FAILURE);                 \
        }                                       \
    } while (0)

/* Thread-safe queues */

#define MAX 10                  /* maximum number of characters in queue */

typedef struct {
    char buffer[MAX];           /* circular buffer */
    size_t in;                  /* position of producer in buffer */
    size_t out;                 /* position of consumer in buffer */
    size_t count;               /* number of unconsumed items */
    pthread_mutex_t mutex;      /* mutex protecting buffer, in, out, count */
    sem_t *empty;               /* semaphore counting empty slots in buffer */
    sem_t *full;                /* semaphore counting full slots in buffer */
} queue_s, *queue_t;

void queue_init(queue_t q)
{
    q->count = 0;
    q->in = 0;
    q->out = 0;

    CHECK(pthread_mutex_init(&q->mutex, NULL) == 0, "pthread_mutex_init");
    q->full = sem_open("full", O_CREAT, 0600, 0);
    CHECK(q->full != SEM_FAILED, "sem_open");
    CHECK(sem_unlink("full") == 0, "sem_unlink");
    q->empty = sem_open("empty", O_CREAT, 0600, MAX);
    CHECK(q->empty != SEM_FAILED, "sem_open");
    CHECK(sem_unlink("empty") == 0, "sem_unlink");
}

void queue_destroy(queue_t q)
{
    CHECK(pthread_mutex_destroy(&q->mutex) == 0, "pthread_mutex_destroy");
    CHECK(sem_close(q->full) == 0, "sem_close");
    CHECK(sem_close(q->empty) == 0, "sem_close");
}

/* Wait, if necessary, for a slot to become available in the queue,
 * and then append the character c. */
void queue_append(queue_t q, char c)
{
    CHECK(sem_wait(q->empty) == 0, "sem_wait");
    CHECK(pthread_mutex_lock(&q->mutex) == 0, "pthread_mutex_lock");

    q->buffer[q->in] = c;
    q->in++;
    q->in %= MAX;
    assert(q->count < MAX);
    q->count++;

    CHECK(pthread_mutex_unlock(&q->mutex) == 0, "pthread_mutex_unlock");
    CHECK(sem_post(q->full) == 0, "sem_post");
}

/* Wait, if necessary, for a slot to become full in the queue, and
 * then pop and return the first character. */
char queue_pop(queue_t q)
{
    CHECK(sem_wait(q->full) == 0, "sem_wait");
    CHECK(pthread_mutex_lock(&q->mutex) == 0, "pthread_mutex_lock");

    char val = q->buffer[q->out];
    q->out++;
    q->out %= MAX;
    assert(q->count > 0);
    q->count--;
    assert(q->count == (q->in + MAX - q->out) % MAX);

    CHECK(pthread_mutex_unlock(&q->mutex) == 0, "pthread_mutex_unlock");
    CHECK(sem_post(q->empty) == 0, "sem_post");
    return val;
}

/* Test harness */

queue_s queue;
char substring[5];

void *producer(void *arg)
{
    char array[] = "HelloWorld";
    size_t i = 0;
    printf("Producer Thread Created\n");
    while (array[i] != '\0') {
        queue_append(&queue, array[i]);
        i++;
    }
    return NULL;
}

void *consumer(void *arg)
{
    size_t i = 0;
    printf("Consumer Thread Created\n");
    for (i = 0; i < 5; i++) {
        substring[i] = queue_pop(&queue);
    }
    return NULL;
}

int main()
{
    pthread_t producer_thread, consumer_thread;
    queue_init(&queue);
    CHECK(pthread_create(&consumer_thread, NULL, consumer, NULL) == 0,
          "pthread_create");
    CHECK(pthread_create(&producer_thread, NULL, producer, NULL) == 0,
          "pthread_create");
    CHECK(pthread_join(consumer_thread, NULL) == 0, "pthread_join");
    CHECK(pthread_join(producer_thread, NULL) == 0, "pthread_join");
    printf("Next character in queue: %c\n", queue.buffer[queue.out]);
    printf("substring: %s\n", substring);
    queue_destroy(&queue);
    return EXIT_SUCCESS;
}


Parsing a complex number using regular expressions Requirements were to create a constructor for a complex number, that receives a string. For example: \$3+5i\$. The regex extracts positive or negative numbers from the string with optional decimal.

My professor told me that my regex was "beyond wrong":

public Complex(String str) {

        ArrayList<Double> list = new
            ArrayList<Double>();

        Pattern p = Pattern.compile("[-+]?[0-9]*\\.?[0-9]");
        // toString() handles the letter i

        // find positive and negative doubles in string and add to list
        Matcher m = p.matcher(str);
        while (m.find()) {
            double num = Double.parseDouble(m.group());
            list.add(num);
        }
        this.re = list.get(0);
        this.im = list.get(1);
}

Here is a link to the class if you need a better understanding of the code.

He then posted his chosen correct solution, which I found to be repetitive. He may argue that I am not checking for i, but I see no point of storing i if we are not using it later.

In addition, if he argues that I am not checking for and storing i, that would mean that if I did store them, I would explicitly have to go exclude the i from the string when I perform mathematical operations on the numbers. It seems counter-productive.

Here is the solution he preferred:

public Complex(String c) {

      String numberNoWhiteSpace = c.replaceAll("\\s","");

      // Matches complex number with BOTH real AND imaginary parts.
      // Ex: -3-2.0i
      Pattern patternA = Pattern.compile("([-]?[0-9]+\\.?[0-9]?)([-|+]+[0-9]+\\.?[0-9]?)[i$]+");

      // Matches ONLY real number.
      // Ex: 3.145
      Pattern patternB = Pattern.compile("([-]?[0-9]+\\.?[0-9]?)$");

      // Matches ONLY imaginary number.
      // Ex: -10i
      Pattern patternC = Pattern.compile("([-]?[0-9]+\\.?[0-9]?)[i$]");

      Matcher matcherA = patternA.matcher(numberNoWhiteSpace);
      Matcher matcherB = patternB.matcher(numberNoWhiteSpace);
      Matcher matcherC = patternC.matcher(numberNoWhiteSpace);

      if (matcherA.find()) {
          real = Double.parseDouble(matcherA.group(1));
          imaginary = Double.parseDouble(matcherA.group(2));
      } else if (matcherB.find()) {
          real = Double.parseDouble(matcherB.group(1));
          imaginary = 0;
      } else if (matcherC.find()) {
          real = 0;
          imaginary = Double.parseDouble(matcherC.group(1));
      }
}

Both of your solutions are wrong, in different ways.  Yours is worse, though, because it fails in a way that can actually succeed in giving completely wrong answers.

Your solution

Your technique is, as your professor noted, sloppy.  It falls apart in several cases that I would consider reasonable inputs:

Pure imaginary numbers such as "5i"
No imaginary component, such as "1"
No imaginary coefficient, such as "5+i"
Imaginary number with a space, such as "3 - 3i"
Decimals more precise than one-tenth, such as "3.14"

Furthermore, using an ArrayList seems extravagant.  You shouldn't need one.

Professor's solution1

Sanitizing the input by discarding spaces — is that a good idea?  Why?  Is there another way to handle spaces?

Why three Patterns, three Matchers, and three .find()s?  Can it be done with fewer?

Within each pattern, is your regex more correct, or his?  Can you spot any bugs in his character classes (i.e. the stuff inside [square brackets])?

Are there any inputs that also fail with his code?  Does his code accept any inputs that should be considered invalid?

1 We shouldn't really be criticizing code that you didn't write, so I'll give you some questions to ponder instead.


Testing extending a list I am learning Python. I have a scenario in my current project to benchmark the following code snippet; by using refactoring to increase the performance. Upon deployment my code will be used on a larger scale.
I'm trying from my side and am also looking for suggestions/guidance from you all. Is there a way to make this code much faster and cleaner with less line of code?
def test(x1, x2):
    if x2 is None:
        return
    elif len(x2) == 1:
        x1.append(x2)
        return
    else:
        for i in range(len(x2)):
            x1.append(x2[i])
    return

l = ['tt', 'yy', 'pp']
test(l, None)
print (l)

test(l, ['hello'])
print(l)

test(l, ['hello', 'world'])
print(l)

Use extend() instead of append() to add a list to another list, and use the fact that None and empty lists compare as False, while a non-empty list compares as True, and omit the redundant return statement:
def test(x1, x2):
    if x2:
        x1.extend(x2)

As for reducing the number of print statements, prepare a list or tuple with your test cases, and handle them all in a single for-loop:
l = ['tt', 'yy', 'pp']
testcases = [
    None,
    ['hello'],
    ['hello', 'world']
]

for testcase in testcases:
    test(l, testcase)
    print(l)


Change the JTextField font and text with radio buttons So I learned from YouTube videos and online tutorials and they gave me a task to do a simple program. I need to pick 1 button from 4 buttons (radio buttons) and each time I pick a button he's changing a text in a text box.

The program is working great ! I'm just asking you guys to check if I did something that is considered wrong in programming or if there's a better way to do this task.

So that's my main program.

import javax.swing.JFrame;

public class Main {

    public static void main(String args[])
    {
        gui go = new gui();
        go.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        go.setSize(317,220);
        go.setVisible(true);
    }

}

and that's the class i'm using (gui class)

package learning;

import java.awt.FlowLayout;
import java.awt.Font;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.ButtonGroup;
import javax.swing.JFrame;
import javax.swing.JRadioButton;
import javax.swing.JTextField;

public class gui extends JFrame implements ActionListener  {

    private JTextField tf = new JTextField( 25);
    private JRadioButton regularRadio = new JRadioButton("Regular",true);
    private JRadioButton boldRadio = new JRadioButton("Bold",false);
    private JRadioButton italicRadio = new JRadioButton("Italic",false);
    private JRadioButton boldNitalicRadio = new JRadioButton("Bold And Italic",false);;
    private ButtonGroup group = new ButtonGroup();

    public  gui(){

        super("Changing text program");
        setLayout (new FlowLayout());

        add(tf);
        add (regularRadio);
        add (boldRadio );
        add (italicRadio);
        add (boldNitalicRadio);

        group.add(boldRadio);
        group.add(boldNitalicRadio);
        group.add(italicRadio);
        group.add(regularRadio);

        regularRadio.addActionListener(this);
        boldRadio.addActionListener(this);
        italicRadio.addActionListener(this);
        boldNitalicRadio.addActionListener(this);

    }

    public void actionPerformed(ActionEvent e){

        if (e.getActionCommand() == "Regular"){
       tf.setText("You picked regular radio button");
       tf.setFont(new Font ("David" , Font.PLAIN , 14));
        }
        else if (e.getActionCommand() == "Bold") {
            tf.setText("You picked bold radio button");
               tf.setFont(new Font ("David" , Font.BOLD , 14));
        }
        else if (e.getActionCommand() == "Italic") {
            tf.setText("You picked italic radio button");
               tf.setFont(new Font ("David" , Font.ITALIC , 14));
        }
        else{
            tf.setText("You picked Bold and Italic radio button");
               tf.setFont(new Font ("David" , Font.ITALIC + Font.BOLD , 14));
        }

    }
}

Java naming convention

You should read the Java naming convention. Class require PascaleCase, so
public class gui should be public class Gui.

Indentation and style

You should use an IDE or a tools to automatically correct any styling or indentation error. The spacing of your variables declarations are not correct and your indentation of your if-else are not correct either.

There are plenty of good free to use IDE and tools. This goes in pair with the Java naming convention/ Java Style recommendation.

Use better names

You're not limited in size for your variable names. Go for clarity over brievty!
This will go a long way to help you, at first glance, know what the variable is about. For the next person that read your code it will be easier too.

private JTextField tf = new JTextField( 25);
private JRadioButton regularRadio = new JRadioButton("Regular",true);
private JRadioButton boldRadio = new JRadioButton("Bold",false);
private JRadioButton italicRadio = new JRadioButton("Italic",false);
private JRadioButton boldNitalicRadio = new JRadioButton("Bold And Italic",false);;
private ButtonGroup group = new ButtonGroup();

Should probably be :

private JTextField textField = new JTextField( 25);
private JRadioButton regularRadio = new JRadioButton("Regular",true);
private JRadioButton boldRadio = new JRadioButton("Bold",false);
private JRadioButton italicRadio = new JRadioButton("Italic",false);
private JRadioButton boldAndItalicRadio = new JRadioButton("Bold And Italic",false);;
private ButtonGroup group = new ButtonGroup();

init

In most GUI code I've seen, you can create an init method and put all the initialization code in there. It helps to keep the constructor clean, but it's no big deals. It's what I'm mostly used too, but it's been a long time I've done any Swing related code.

Constant

If you see a value used over and over, ask yourself : Does this value represent the same thing?

Take "David" for example, it does represent the same named font in all your case. You could then have a private static String DAVID = "David" to represent the font.

Action command

This is a special case from your previous point. Since you hard-coded the action command names, if you change it at one place you have to remember to change it in other places.

This is a perfect example of the Java Enum class. Take a look at https://docs.oracle.com/javase/tutorial/java/javaOO/enum.html, it's a good tutorial on how to use it.

Using an enum you make it clearer and should be easier to use in the long run.

Other point

I don't think you need to create the Font object every time, you may use an const Font object to reduce object creation. It's a small thing that not improve really but it could help in other cases.


Loading... animating dots in C I've recently wanted to make a "Loading..." display in C where the dots print one at a time in order and then reset:

Suprisingly, there isn't much on the internet for doing this well, so I figured I would make a simple program for it.

#include <stdio.h>
#include <time.h>

int main(void)
{
    int msec = 0;
    const int trigger = 500; // ms
    const int printWidth = 4;
    int counter = 0;
    clock_t before = clock();

    while (1)
    {
        fputs("Loading", stdout);
        clock_t difference = clock() - before;
        msec = difference * 1000 / CLOCKS_PER_SEC;
        if (msec >= trigger)
        {
            counter++;
            msec = 0;
            before = clock();
        }
        for (int i = 0; i < counter; ++i)
        {
            fputc('.', stdout);
        }
        for (int i = 0; i < printWidth - counter; ++i)
        {
            fputc(' ', stdout);
        }
        fputc('\r', stdout);
        fflush(stdout);

        if (counter == printWidth)
        {
            counter = 0;
        }
    }
}

The output given is seen in the .gif above.

I know this can be done better.  Any suggestions for improvement?

Too frenetic

I ran your program but it was very frenetic.  It was constantly clearing the "Loading" prompt and reprinting it which resulted in a flickering effect.  In addition, the cursor also moved around in a flickery manner (similar to the green box in the animated image).

To improve this, I would do two things:

Don't constantly draw when nothing has changed.  Instead, sleep until the next trigger time and then redraw.  This also has the added benefit of not using 100% of your cpu.  Presumably, you will have another thread running which is doing some loading work, and you don't want this thread to hog cpu time.
You don't need to clear and redraw the whole prompt until you get to the last dot.  Up until that point, you can just draw one dot at a time.

Rewrite

I rewrote your program with the above two fixes in mind:

#include <stdio.h>
#include <unistd.h>

int main(void)
{
    const int trigger = 500; // ms
    const int numDots = 4;
    const char prompt[] = "Loading";

    while (1) {
        // Return and clear with spaces, then return and print prompt.
        printf("\r%*s\r%s", sizeof(prompt) - 1 + numDots, "", prompt);
        fflush(stdout);

        // Print numDots number of dots, one every trigger milliseconds.
        for (int i = 0; i < numDots; i++) {
            usleep(trigger * 1000);
            fputc('.', stdout);
            fflush(stdout);
        }
    }
}

Rewrite 2

In response to the comment where @syb0rg indicated that the main thread should not sleep, here is what I would do in that case:

#include <stdio.h>
#include <time.h>

static void redrawPrompt(void);
static void doWork(void);

int main(void)
{
    const int trigger   = (CLOCKS_PER_SEC * 500) / 1000;  // 500 ms in clocks.
    clock_t   prevClock = clock() - trigger;

    while (1) {
        clock_t curClock = clock();

        if (curClock - prevClock >= trigger) {
            prevClock = curClock;
            redrawPrompt();
        }
        doWork();
    }
}

static void redrawPrompt(void)
{
    static int  numDots;
    const  int  maxDots = 4;
    const  char prompt[] = "Loading";

    // Return and clear with spaces, then return and print prompt.
    printf("\r%*s\r%s", sizeof(prompt) - 1 + maxDots, "", prompt);
    for (int i = 0; i < numDots; i++)
        fputc('.', stdout);
    fflush(stdout);
    if (++numDots > maxDots)
        numDots = 0;
}

static void doWork(void)
{
    // This function does loading work but returns at least every 500 ms.
}


Calculator for area and perimeter of a window This program calculates wood length and area of glass required for the window.
I am a beginner in C# and I want to know how professional programmers will write program like this.

        double width, height, woodLength, glassArea;
        const double MAX_WIDTH = 5.0;
        const double MIN_WIDTH = 0.5;
        const double MAX_HEIGHT = 3.0;
        const double MIN_Height = 0.75;
        string widthString, heightString;
        Console.WriteLine("please enter width of the window");
       widthString = Console.ReadLine();
        width = Convert.ToDouble(widthString);
        if (width < MIN_WIDTH)
        {
            Console.WriteLine("you enter vaule less than min width \n"+ "using mininium one");
            width = MIN_WIDTH;
        }
        if (width>MAX_WIDTH)
        {
            Console.WriteLine("you entered value grater than MaX width\n" + "using maxuimum one");

            width = MAX_WIDTH;
        }
        Console.WriteLine("please enter your height of the window");
        heightString = Console.ReadLine();
        height = double.Parse(heightString);
        if (height<MIN_Height)
        {
            Console.WriteLine("you enter vaule less than min width \n" + "using mininium one");
            height = MIN_Height;

        }
        if (height > MAX_HEIGHT)
        {
            Console.WriteLine("you enter value grater than Max height\n" + "using maxuimum one");
            height = MAX_HEIGHT;
        }
        woodLength = 2 * (width+height);
        glassArea =  (width * height);
        Console.WriteLine("the lenth of the wood is "+woodLength+"feet");
        Console.WriteLine("the area of the glass is " + glassArea +"m^2");

Typos / Spelling issue

Attention to detail is important.  Spelling errors, in variable names + strings should be avoided.  They give a bad impression.  You have several.  mininium should be minimum, maxuimum should be maximum, Sentences should start with a capital letter etc.

Be clear with the user

You ask the user for the width/height of the window, however you don't tell the user what unit the measurements are in. Your original code you were converting to feet (by multiplying by 3.25).  If you're going to do something like this, consider defining a constant with a name that reflects what it is, FeetPerMetre for example (Constants should be in Pascal, not ALL_CAPS naming).  You also haven't told the user the min/max values either when you prompt for input, or when you decide to use them.

Contain your functionality

The code you've posted is all in a single block.  It doesn't appear to have been wrapped in a function, suggesting that it's just sitting in a static main somewhere.  Think about what elements the code contains and break it out into functions that you might be able to reuse.  Some possible functions might be:

int GetLengthFromUser(string prompt, double minLength, double maxLength)
double CalculateWindowFrameLength(double width, double length)
double CalculateWindowPaneArea(double width, double length)
double ConvertFeetToMetres(double length)

Interpolated Strings

Depending on the C# version you're using, you may be able to use interpolated strings which can things like WriteLine statements look a bit cleaner:

Console.WriteLine($"The lenth of the wood is {woodLength} feet");

double.Parse / Convert.ToDouble

Both methods can fail and throw an exception if the user enters something other than a number (which will inevitably happen eventually).  Consider using TryParse and re-prompting the user when it fails.


Write a function named isSuper that returns 1 if its array argument is a isSuper array, otherwise it returns 0 An Super array is defined to be an array in which each element is greater than sum of all elements before that.

For example:

{2, 3, 6, 13} is a Super array. (2 < 3, 2+3 < 6, 2 + 3 + 6 < 13)
{2, 3, 5, 11} is a NOT a Super array. 2 + 3 < 5 is false.

I wrote following code to find the array is Super Array or not .

public class SuperArray {
    public static void main(String args[]) {
         System.out.println("The result is: " + isSupper(new int[] {2, 3, 6, 13}));
         System.out.println("The result is: " + isSupper(new int[] {2, 3, 5, 11}));
    }
    public static int isSupper(int[] a){
        int result = 0;
        int sum = 0;
        int nextValue = 0;
        for(int i = 0; i < a.length; i++) {
            nextValue = a[i];
            if(nextValue > sum) {
                result = 1;
            } else {
                result = 0;
                break;
            }
            System.out.println("sum is: " + sum);
            System.out.println("next Value is: " + nextValue);
            sum += a[i];
        }
        return result;
    }
}

Is this a good way to find Super array? First a assign sum = 0 and check that sum to first number of an array and so on.

It's easier to use the enhanced for loop like this:

    for(int value : a) {
        if(value <= sum)
            return 0;
        sum += value;
    }
    return 1;

This also clears up some unnecessary lines like checking if the function should break and return. When you return a value, it immediately leaves the function. Therefore, you don't need to break then return. If all value pass the for loop, then it will return 1.

Also, you might want to return boolean instead of int in your method because it's just true or false.


99 beers song in Rust I'm learning Rust.

I find it sometimes confusing when compared to other programming languages, especially when dealing with strings and slices.

Her's an implementation of the 99 beer song that I've written, but the code feels clunky and I think it's bad.

How would you have written it, if it was a production code of a real-world software? Any way to improve performance, readability and a more reasonable way to deal with strings in Rust?

const BOTTLE: &str = "bottle";
const S: &str = "s";
const OF_BEER: &str = "of beer";
const N_UP: &str = "N";
const N_DOWN: &str = "n";
const O_MORE: &str = "o more";
const ON_THE_WALL: &str = "on the wall";
const SPACE: &str = " ";
const COMMA: &str = ",";
const PERIOD: &str = ".";
const NL: &str = "\n";
const TAKE: &str = "Take";
const IT: &str = "it";
const ONE: &str = "one";
const DOWN_AND_PASS: &str = "down and pass it around";
const GO_STORE: &str = "Go to the store and buy some more";
const NINE_NINE: &str = "99";

pub fn verse(n: i32) -> String {
    let mut phrase: String;
    let bottles = &format!("{}{}", BOTTLE, S)[..];
    let bottles_of_beer = &format!(
        "{}{}{}",if n == 1 { BOTTLE } else { bottles }, SPACE, OF_BEER
    );
    let n_str = &format!("{}", n)[..];

    if n < 1 {
        phrase = format!("{}{}{}{}", N_UP, O_MORE, SPACE, bottles_of_beer);
    } else {
        phrase = format!("{}{}{}", n_str, SPACE, bottles_of_beer);
    }

    phrase = phrase + SPACE + ON_THE_WALL + COMMA + SPACE;

    if n < 1 {
        phrase = phrase + N_DOWN + O_MORE;
    } else {
        phrase = phrase + n_str;
    }

    phrase = phrase + SPACE + bottles_of_beer + PERIOD + NL;

    if n < 1 {
        phrase = phrase + GO_STORE;
    } else {
        phrase = phrase + TAKE + SPACE + if n == 1 { IT } else { ONE } + SPACE + DOWN_AND_PASS
    }

    phrase = phrase + COMMA + SPACE;

    if n < 1 {
        phrase = phrase + NINE_NINE + SPACE + bottles;
    } else {
        let n = n - 1;

        if n < 1 {
            phrase = phrase + N_DOWN + O_MORE + SPACE + bottles;
        } else {
            phrase = phrase + &format!("{}", n)[..] + SPACE + if n == 1 { BOTTLE } else { bottles };
        }
    }

    phrase = phrase + SPACE + OF_BEER + SPACE + ON_THE_WALL + PERIOD + NL;

    return phrase;
}

pub fn sing(start: i32, end: i32) -> String {
    let mut s = String::new();
    let mut x = start;

    while x >= end {
        if x < start {
            s = s + NL;
        }
        s = s + &verse(x)[..];
        x -= 1;
    }

    return s
}

Being myself new to rust, take the following with a grain of salt. I would welcome further edits.

I think, generally, that there are two things your code is trying to do.
On one hand, it's about string tokens and concatenation.
On the other, there's the solving the 99 bottles problem.

99 Bottles

For this problem, your approach seems complex and results in rather gauche
code. What makes it so, is that it's hard to understand at first glance what
phrase is at any point.
Adding comments would be a good thing.
Myself, I would forego the whole tokenisation part, and solve it in a more
concise way.

There are only a handful of sentences in the song, which need slight variations.
I'd like to use something like Python's raw strings:

SOLO = r"{QTY} bottles of beer on the wall, {QTY} bottles of beer."
SOLOT.format(QTY=count)

In rust, I managed to get this behaviour by using std::String::replace:

const SOLO: &str = "{QTY} bottles of beer on the wall, {QTY} bottles of beer."
let mut solo: String = SOLO.to_string().replace("{QTY}", &count.to_string());

Once can likewise replace bottles for its singular form, one for it.

Other details to clarify are:
1. are negative bottles allowed?
2. in one place, we're talking of phrase, in another of verse, and returned are
a bunch of sentences. Which nomenclature is correct? A single term ought to be used.
3. We're only dealing with beer, perhaps bottles_of_beer could become bottles?

We then end up with something like so:

const SOLO: &str = "{QTY} bottles on the wall. ";
const CHORUS: &str = "Take one down and pass it around, {QTY} bottles of beer on the wall.\n";
const BOTTLE: &str = "bottle"; // for substitution when 1 bottle left.
const NONE: &str = "\
No more bottles of beer on the wall, no more bottles of beer.\n\
Go to the store and buy some more, {QTY} bottles of beer on the wall.\n\
";

pub fn get_song(start: u32, stop: u32) -> String {
    let mut song: String = String::new();

    for x in (start..stop+1).rev() {
        if x == 0 {
            let no_beer: String = NONE.to_string().replace("{QTY}", &start.to_string());
            song = song + &no_beer;
            break;
        }
        let mut solo: String = SOLO.to_string().replace("{QTY}", &x.to_string());
        let mut chorus: String = CHORUS.to_string().replace("{QTY}", &x.to_string());
        if x == 1 {
            solo = solo.replace("bottles", BOTTLE);
            chorus = chorus.replace("bottles", BOTTLE);
            chorus = chorus.replace("one", "it");
        }
        song = song + &solo + &chorus;
    }
    return song;
}

fn main() {
   println!("{}", get_song(10, 0));
}

note that I am not sure this is a correct way to do such constant
strings.
I think let mut song: String = String::new(); is the preferred style,
as seen in the rust book, over
let mut phrase: String;

The while loop can be replaced by a for loop. This is actually the textbook example:

for x in (stop..start+1).rev() {
    ...
}

There's the +1 increment to include the starting value, then the iterator is reversed to go from high to low values.

String Tokens and Concatenation

To stay on the string concatenation path, I suggest commenting what each path does.
There are several if n < 1, which could be put together.  Within that, n
is redefined and subtracted, leading to an nested if n < 1 test.

We can observe that there are two types of operations: building different types of sentences,
and checking whether we're at the end of the song (n < 1).

Let's agree to not return a phrase, but a paragraph:

let mut paragraph: String = String::new();

// Build qty of bottles on the wall sentence
if n < 1 {
  ...
  paragraph = format!(...);
} else { ... }

// Build qty of bottles left on the wall sentence
if n < 1 {
   paragraph = format!("{} {}", paragraph, ...);
} else { ... }

And so on.

Now I'm thinking, what's actually done here, is not checking a condition, but
rather matching a parameter, so if-elses could become:

// Build qty of bottles on the wall sentence
match n {
   0 => paragraph = format!("{}{}{}{}", N_UP, O_MORE, SPACE, bottles),
   _ => paragraph = format!("{}{}{}", n_str, SPACE, bottles),
}

By using match extensively, we also can remove the nested if-else:

    // Build closing
    let phrase = match n {
        0 => format!("{}{}{}", NINE_NINE, SPACE, bottles),
        1 => format!("{}{}{}{}", N_DOWN, O_MORE, SPACE, bottles),
        _ => format!("{}{}{}", qty, SPACE, bottles),
    };

Finally, we get to such a result:

const BOTTLE: &str = "bottle";
const BOTTLES: &str = "bottles";
const OF_BEER: &str = "of beer";
const UPPERCASE_N: &str = "N";
const LOWERCASE_N: &str = "n";
const O_MORE: &str = "o more";
const ON_THE_WALL: &str = "on the wall";
const SPACE: &str = " ";
const COMMA: &str = ",";
const PERIOD: &str = ".";
const NL: &str = "\n";
const TAKE: &str = "Take";
const IT: &str = "it";
const ONE: &str = "one";
const DOWN_AND_PASS: &str = "down and pass it around";
const GO_STORE: &str = "Go to the store and buy some more";
const NINE_NINE: &str = "99";

pub fn verse(n: u32) -> String {
    let mut paragraph: String = String::new();

    // Is `bottle` plural in this paragraph?
    let bottles: String = match n {
        1 => BOTTLE.to_string(),
        _ => BOTTLES.to_string(),
    };

    // Create first half of first verse.
    let phrase: String = match n {
        0 => UPPERCASE_N.to_owned() + O_MORE + SPACE + &bottles,
        _ => n.to_string() + SPACE + &bottles,
    };
    paragraph = paragraph + &phrase;
    paragraph = paragraph + SPACE + ON_THE_WALL + COMMA + SPACE;

    // Create second half of first verse.
    let phrase: String = match n {
        0 => LOWERCASE_N.to_owned() + O_MORE,
        _ => n.to_string(),
    };
    paragraph = paragraph + &phrase;
    paragraph = paragraph + SPACE + &bottles + PERIOD + NL;

    // Create first half of second verse.
    let phrase: String = match n {
        0 => GO_STORE.to_string(),
        1 => TAKE.to_owned() + SPACE + IT + SPACE + DOWN_AND_PASS,
        _ => TAKE.to_owned() + SPACE + ONE + SPACE + DOWN_AND_PASS,
    };
    paragraph = paragraph + &phrase;
    // Add punctuation
    paragraph = paragraph + COMMA + SPACE;

    // Create second half of second verse.
    let phrase = match n {
        0 => NINE_NINE.to_owned() + SPACE + &bottles,
        1 => LOWERCASE_N.to_owned() + O_MORE + SPACE + BOTTLES,
        _ => n.to_string() + SPACE + &bottles,
    };
    paragraph = paragraph + &phrase;

    // Finish second verse
    paragraph = paragraph + SPACE + OF_BEER + SPACE + ON_THE_WALL + PERIOD + NL;

    return paragraph;
}

fn main() {
    let start: u32 = 10;
    let stop: u32 = 0;
    let mut song: String = String::new();
    for x in (stop..start+1).rev() {
        song = song + &verse(x);
    }
    println!("{}", song);
}

This is still not too clear code to me, and I'd rather work with the
first variant.

On a final note, there were places were you use n_str or format!("{}", n), be mindful of this types of errors.
Likewise, typically start/stop or begin/end are used, but try not to mix them up.
Lastly, I had a lot of trouble distinguishing between O_DOWN N_DOWN, N_UP.


Using decorator pattern to validate an entity I am following this article.

/**
 * The group name should contain a minimum of 3 and a maximum of 12 capital letters.
 * No space, number, or special characters are allowed
 */

console.clear();

class Group {
  constructor(name) {
    this.name = name;
  }

  getName() {
    return this.name;
  }

  getLength() {
    return this.name.length;
  }

  add(name) {}

  remove(name) {}

  toString() {
    return `Group: ${this.name}`;
  }
}

class DefaultGroup {
  constructor(group) {
    this.origin = group;
  }

  getName() {
    return this.origin.getName();
  }

  getLength() {
    return this.origin.getLength();
  }

  validate() {
    return true;
  }
}

class OnlyUpCaseGroup extends DefaultGroup {
  constructor(origin) {
    super(origin);

    if (!this.validate()) {
      alert('Invalid group name ' + this.getName());
      throw Error('Invalid group name');
    }
  }

  validate() {
    let REGEX_UP_CASE = /^[A-Z]*$/g;
    return super.validate() && REGEX_UP_CASE.test(this.getName());
  }

  toString() {
    return `Group: ${this.getName()}`;
  }
}

class ValidLengthGroup extends DefaultGroup {
  constructor(origin) {
    super(origin);

    if (!this.validate()) {
      alert('Length must be between 3 and 12');
      throw Error('Invalid group name');
    }
  }

  validate() {
    let len = this.getLength();
    return super.validate() && len >= 3 && len <= 12;
  }

  toString() {
    return `Group: ${this.getName()}`;
  }
}

let g = new Group('FOOOBAAAR');
//console.log(new OnlyUpCaseGroup(new Group('vivek')));
//console.log(new OnlyUpCaseGroup(new DefaultGroup(g)));
//console.log(new ValidLengthGroup(new DefaultGroup(g)));
console.log(new ValidLengthGroup(new OnlyUpCaseGroup(new DefaultGroup(g))));

Questions:

I need to know if I am following the pattern properly?
Can I do same with less verbose code?

Note:

Maintainability and flexibility is prime here. Keep in mind that there can be more validations.

Unfortunately ecmascript-6 does not have support for interfaces yet. What this means is that Group does not quite make sense and that DefaultGroup should be your Group instead. No matter how I look at it I don't really see any kind of usefulness on having both of those classes existing.

If we extrapolate the decorator pattern to one of the most common examples in C# present in Stream classes (StreamReader, BufferedStream, ...) you will clearly see that there is no DefaultStream.

It doesn't even exist a common interface. I am not saying that is the best and that is always what you should do. What you should do is to think about what makes sense on your language and in your requisites.

Keep going with this conversation you are doing your architecture. But did you reach to the conclusion that decorator pattern was really the best choice?

Could strategy pattern, together with composite pattern be a better alternative?

If I had to answer this question I would probably answer: Yes it would.

Let me explain why:

The decorator exists in order to add some kind of functionality that your class didn't have before. It's true that in your use case you are adding validation, a thing that you didn't have before. However what you really want to do is to have multiple kind of validations (strategies). You then could use all strategies you wanted by putting them on a list, iterating over them and call the validation method (this is the composite pattern)

Not to say that Group now becomes ignorant about which kind of validation is being made. Which might be a good thing or a bad thing. If you really want your group to be aware of validation it should take a parameter on the constructor to tell him how the validation should be made.

I could keep going by asking why you have hard-coded values for the minimum and maximum length (3 and 12 respectively). And also why do you have a getLength method that does getName().length?

In case you were wondering all of this that I said would translate in the following implementation

class Group {
  constructor(name) {
    this.name = name;
  }

  getName() {
    return this.name;
  }

  toString() {
    return `Group: ${this.name}`;
  }
}

class Validation {
  constructor() {
  }

  validate(value) {
    return true;
  }
}

class OnlyUpCaseValidaton extends Validation {
  constructor() {
    super();
  }

  validate(value) {
    let REGEX_UP_CASE = /^[A-Z]*$/g;
    return REGEX_UP_CASE.test(value);
  }
}

class LengthValidation extends Validation {
  constructor(minLength, maxLength) {
    super();
    this.minLength = minLength || 3;
    this.maxLength = maxLength || 12;
  }

  validate(value) {
    let len = value.length;
    return len >= this.minLength && len <= this.maxLength;
  }
}

let g = new Group('FOOOBAAAR');
//poor man's composite...
var validations = [new LengthValidation(), new OnlyUpCaseValidaton()];
console.log(validations.every(function(validation){
    return validation.validate(g.getName())
}));


JavaScript calculator using inputs value I’m trying to make a simple calculator with 3 inputs in JavaScript but it doesn’t work. For now my code is:

<input placeholder="Number" id="Number F” class="Number F" onchange="calculator()">
<input placeholder="/ ,  * ,  -  or +" id="Number O" class="Number O" onchange="calculator()">
<input placeholder="Number" id="Number S" class="Number S" onchange="calculator()">

<button id="button" onclick="calculator()"></button>

I’m beginner in coding. What is wrong? Please Help Me! Thank you!

Id attribute cannot contains spaces.

You can also use an object which contains the function's definition for all the possible operations.

var operators = {
  '+': function(a, b) { return +a + (+b) },
  '-': function(a, b) { return a - b },
  '/': function(a, b) { return a / b },
  '*': function(a, b) { return a * b },
};

var operators = {
    '+': function(a, b) { return +a + (+b) },
    '-': function(a, b) { return a - b },
    '/': function(a, b) { return a / b },
    '*': function(a, b) { return a * b },
};
function calculator() {
  var cel = document.getElementById("NumberF").value;
 var celular = document.getElementById("NumberS").value;
  var operator = document.getElementById("NumberO").value;
  var result = document.getElementById("resul");
  if(cel && celular && operator)
    result.innerHTML = operators[operator](cel, celular);
};
<input placeholder="Number" id="NumberF" class="NumberF" onchange="calculator()">
<input placeholder="/ ,  * ,  -  or +" id="NumberO" class="NumberO" onchange="calculator()">
<input placeholder="Number" id="NumberS" class="NumberS" onchange="calculator()">

<button id="button" onclick="calculator()">Click</button>


Return fake jqXHR from custom Ajax method I have several methods that wrap the $.ajax() method and return its jqXHR, but I need to force fail before even calling the original $.ajax() and still allow to use success/error methods of the result.

//just a very simple example, actuall code is more complicated
$.rpc = function(method, args) {

    if (!method || 'string' === typeof method ||) {
        return false; //PROBLEM - cannot use success/error handlers!
    }

    return $.ajax({
        type: 'POST',
        url: '/rpc',
        data: { method: method, params: args },
    }); //AJAX()
};

//usage:
$.rpc(loginMethod, credentials).success(gotoProfile).fail(gotoLogin);
//if loginMethod is for any reason undefined it crash instead of calling fail handler

So I use this helper method, since the jqXHR is created directly inside the Ajax method:

$.ajax.getFailed = function(message) {
    var
        jqDef = new $.Deferred(),
        jqXHR = jqDef.promise();

    jqXHR.success = jqXHR.done; //deprecated  $.ajax methods
    jqXHR.error = jqXHR.fail;

    jqDef.reject.defer(1, jqXHR, [jqXHR, message]);
               //defer is from extJS, alternative to setTimeout

    return jqXHR;
};

$.rpc = function(method, args) {

    if (!method || 'string' === typeof method ||) {
        return $.ajax.getFailed('Missing method');
    }

    return $.ajax({
        type: 'POST',
        url: '/rpc',
        data: { method: method, params: args },
    }); //AJAX()
};

Is this code OK or is there anything else I should add?

Looking at the documentation for $.ajax reveals a beforeSend option that would allow you to cancel the request before it's sent by returning false.

$.rpc = function(method, args) {
    var beforeSend = function () {
        return method && 'string' !== typeof method;
    }

    return $.ajax({
        type: 'POST',
        url: '/rpc',
        data: { method: method, params: args },
        beforeSend: beforeSend
    });
};


Periodically sending data from database to an API I'm trying to make a timed background service in .NET Core (I found this approach in Microsoft Docs), that every 15 seconds it gets data from the database (1000 rows max, if something exists) then sends it to the API using a foreach loop. If a response status code is not 200 or 202, I change thr status column by id in the database.

How can I make this code faster? Should I use another approach?

using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using Newtonsoft.Json;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Net.Http;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace FDProcessor
{
    class FDSenderService : IHostedService, IDisposable
    {
        private Timer timer;
        private HttpClient httpClient = new HttpClient
        {
            BaseAddress = new Uri("https://example.com")
        };

        public Task StartAsync(CancellationToken cancellationToken)
        {
            Console.WriteLine("Service is started.");
            timer = new Timer(DoWork, null, TimeSpan.Zero, TimeSpan.FromSeconds(15));
            return Task.CompletedTask;
        }

        private async void DoWork(object state)
        {
            var data = DbManager.GetTransactions();

            foreach (var r in data)
            {
                StringContent jsonRequestBody = new StringContent(JsonConvert.SerializeObject(r));
                var response = await httpClient.PostAsync("/example/path", jsonRequestBody);

                if (!response.IsSuccessStatusCode)
                {
                    Console.WriteLine("Bad request. Status returned.");
                    Console.WriteLine(await response.Content.ReadAsStringAsync());
                    DbManager.UpdateTransaction(null, r.TrnID);
                }
            }
        }

        public Task StopAsync(CancellationToken cancellationToken)
        {
            Console.WriteLine("Stopping!");
            timer?.Change(Timeout.Infinite, 0);

            return Task.CompletedTask;
        }

        public void Dispose()
        {
            timer.Dispose();
        }
    }
}

If Dowork takes longer than 15 seconds to execute, it will be called a second time.  Will this cause issues?  One way around that would be to disable the timer while you're doing work, then start it up again when the work is done, or add an appropriately typed and thread safe "busy" member to the class so that you can skip starting any new work if work is currently being done.


Check if 2 arrays have (exactly) the same elements recursively I've been given a homework assignment to make a function to check whether 2 given arrays with the same given size have exactly the same set of elements.

My function seems to be working but I feel like I don't use the principles of recursion properly in my function.

I'm not allowed to use dictionary like data structures or sorts of all kinds.

Could you think of a simpler recursive approach to this problem?

bool sameElements(int Arr1[], int Arr2[], int size) {

    bool found; // flag to indicate whether element from Arr1 has been found in Arr2

    found = false;

    //base case
    if (size == 1) {

        if (Arr1[0] == Arr2[0])
            return true;
        else
            return false;
    }//if
    else { //size != 1
        int i; //index for loop

        for (i = size - 1; i >= 0 && !found ; i--) {

            if (Arr2[i] == Arr1[size-1]) { // check existence of element in Arr1
                swapInArray(Arr2, i, (size - 1)); // swap elements
                found = true;

            }//if

        }//for

        if (found)
            sameElements(Arr1, Arr2, size - 1); // send to recursion with size-1
        else
            return false;

    }//else
}//sameElements

void swapInArray(int arr[], int i, int j) {
    int temp; // temp value to hold arr[i]
    temp = arr[i];
    arr[i] = arr[j];
    arr[j] = temp;
 }//swapInArray

Boolean return anti-pattern

It is a very common over-complication beginner make to write:

if (cond) {
    return true;
else {
    return false;
}

When all you need is:

return cond;

So the block:

if (Arr1[0] == Arr2[0])
    return true;
else
    return false;

becomes:

return Arr1[0] == Arr2[0];

Never comment when a block closes

Things like:

}//else

are not ok. If you need them to keep track of the code, your code has overly long blocks, and the problem must be fixed at its source.


Splitting a String and Returning Converted Data I have 2 Classes, the first one is only used as Container,
and the second Class Splits a GpsString, Converts it to suitable data types and returns the Container Class.

The String to analyze looks about like this:

("$GPGGA,120003.533,7926.184,N,02222.419,W,1,12,1.0,88.5,M,0.0,M,,*71")

Now i want to know if this is good practice in ObjectOriented Programming,
or if there is a better way for this Problem.

final class GpsData {
    int Hours;
    int Minutes;
    double Seconds;
    double Lattitude;
    char DirectionLattitude; // North or South
    double Longitude;
    char DirectionLongitude; // North or South
    int Quality;
    int Satelites;
}

class ProtocolAnalyzer {
    private GpsData AnalyzedData = new GpsData();
    boolean AnalyzeData(String GpsData){
        GpsData Gps = new GpsData();
        int Attribute = 0;
        StringBuilder WorkString = new StringBuilder();
        int beginIndex = 0;
        int length = GpsData.length();
        for (int i = 0;i<=length;i++){
            if(i == GpsData.length() ||
            GpsData.charAt(i) == ','){
            String Data = GpsData.substring(beginIndex,i);
            boolean bool = ConvertData(Data,Attribute,Gps);
            if(!bool) return false;
            beginIndex = i + 1;
            Attribute ++;
        }
    }
    setAnalyzedData(Gps);
    return true;
}
private boolean ConvertData(String Data,int Attribute,GpsData Gps){
boolean b = false;
switch (Attribute) {
    case 0: // §GPGGA
        if(Data.equals("$GPGGA")) {
            b = true;
        }
        break;
    case 1: //HHMMSS.sss
        if(Data.length() == 10) {
            b = true;
            String split = Data.substring(0,2);
            Gps.Hours = Integer.parseInt(split);
            split = Data.substring(2,4);
            Gps.Minutes = Integer.parseInt(split);
            split = Data.substring(4,10);
            Gps.Seconds = Double.parseDouble(split);
        }
        break;
    case 2: //BBBB.BBBB
        if (Data.length() != 0) {
            b = true;
            Gps.Lattitude = Double.parseDouble(Data);
        }
        break;
    case 3: //b
        if (Data.length() != 0) {
            char DirLat = Data.charAt(0);
            if (DirLat == 'N' ||
                    DirLat == 'S') {
                b = true;
                Gps.DirectionLattitude = DirLat;
            }
        }
        break;
    case 4: //LLLLL.LLLL
        if (Data.length() != 0) {
            b = true;
            Gps.Longitude = Double.parseDouble(Data);
        }
        break;
    case 5: //l
        if (Data.length() != 0) {
            char DirLong = Data.charAt(0);
            if (DirLong == 'E' ||
                    DirLong == 'W') {
                b = true;
                Gps.DirectionLongitude = DirLong;
            }
        }
        break;
    case 6: //Q
        if (Data.length() != 0) {
            int Quality = Integer.parseInt(Data);
            Gps.Quality = Quality;
            if(Quality != 0) {
                b = true;
            }
        }
        break;
    case 7: //NN
        if (Data.length() != 0) {
            int Satellites = Integer.parseInt(Data);
            Gps.Satelites = Satellites;
            if(Satellites > 3) {
                b = true;
            }
        }
        break;
}
return b;
}

private void setAnalyzedData(GpsData LogData) {
    this.AnalyzedData = (LogData);
}

GpsData getAnalyzedData() {
    return AnalyzedData;
}
}

First, please please please use Java naming conventions to aid readability. Methods and local variables like int Hours should be in camelCase and not CapitalCase which is for classes. Statements like this:

boolean AnalyzeData(String GpsData){

are especially confusing. Is GpsData referring to the local variable, or the class? That can get very nasty very quickly. Please fix this before you do anything else.

final class GpsData {

This does not do what I suspect you think it does. A final declaration for a class just means it can't be further extended. Its data can still be modified. To properly encapsulate the properties of GpsData you should declare each of those properties as private and provide public constructors, setters, and getters as appropriate.