Become a MacRumors Supporter for $50/year with no ads, ability to filter front page stories, and private forums.

erdinc27

macrumors regular
Original poster
Jul 20, 2011
168
1
I have been rejected from a company that i applied for a job and sent an example application for them. The feedback was like that.
The codebase was bad, no comment, no unit test and no usage of third parties.
Bad styling and no physical layout-ing.
1- What means codebase here ? The example has a View controller with 3 buttons only.
2- Is there a source that i can learn unit test in a fast way ?
3- Do you put comments often while you are coding ? I generally use pragma mark
4- What means bad styling ?
5- What means no physical layouting ?
Here is the simple application i developed for them.
https://github.com/erdinckolukisa/GoEuro

Can you criticise my code please ?
 
Codebase is the entire project as a whole. It's not good practice to have all your code in a single ViewController. Break out your code into different class that handle specific types of things.

There are a lot websites that explain Unit Testing is how to do it.

Comments should be used when necessary. Don't comment every line, but comment not obvious things and a general overview of the purpose of the class.

Styling could be a few things but most likely, variable/function naming, spacing, the overall structure and appearance of your code. This might seem silly but it makes a HUGE difference when working in a team. It's not fun reading poorly formatted code.

It looks like you aren't using Storyboards. Storyboard are Apples preferred way of laying out how Views should look.

After a quick skim of your project, they are correct in saying your codebase is poor. You have commented out lines present, huge sections of code that isn't used and simply commented out. Your spacing is all over the place with large gaps between some lines of code. I didn't see a Storyboard file in your project either, most companies want someone who is familiar with AutoLayout.
 
Codebase is the entire project as a whole. It's not good practice to have all your code in a single ViewController. Break out your code into different class that handle specific types of things.

There are a lot websites that explain Unit Testing is how to do it.

Comments should be used when necessary. Don't comment every line, but comment not obvious things and a general overview of the purpose of the class.

Styling could be a few things but most likely, variable/function naming, spacing, the overall structure and appearance of your code. This might seem silly but it makes a HUGE difference when working in a team. It's not fun reading poorly formatted code.

It looks like you aren't using Storyboards. Storyboard are Apples preferred way of laying out how Views should look.

After a quick skim of your project, they are correct in saying your codebase is poor. You have commented out lines present, huge sections of code that isn't used and simply commented out. Your spacing is all over the place with large gaps between some lines of code. I didn't see a Storyboard file in your project either, most companies want someone who is familiar with AutoLayout.

Thank you for your reply man. Actually i have a storyboard file in project. Maybe you didnt see it.
I am looking for about unit testing now.
Do you think i have a wrong way about naming variables and functions ? I try to follow camel casing rule.
And last thing how could u break that project into small peaces ? Could u give me an example ?
I tried to break one peace parsing source and another peace to turn raw data to objects. Isnt it enough ?
 
Thank you for your reply man. Actually i have a storyboard file in project. Maybe you didnt see it.
I am looking for about unit testing now.
Do you think i have a wrong way about naming variables and functions ? I try to follow camel casing rule.
And last thing how could u break that project into small peaces ? Could u give me an example ?
I tried to break one peace parsing source and another peace to turn raw data to objects. Isnt it enough ?

You're right I didn't look in the folder for the storyboards, missed it.

So far your naming is ok, but try and be more descriptive in your naming, e.g. "theButton" doesn't describe what the button is for. It's more than just using "camelCase". Naming methods and variables is one of the most important things in Software Engineering. It can make something extremely easy to maintain and understand or it can make it a nightmare where nothing makes sense.

Some pointer for your "selectTheType" Action, try and avoid iterating through your view hierarchy when searching for an item. Avoid using tags to find objects. Not in all cases, but there is usually a better way to solve a problem.

From what I see you've broken your code out fine, but I don't know what the coding requirements are so you may have missed something they wanted.

The biggest most blatant problem is how messy and disorganized your classes are. Don't keep your commented out code in the class and maintain consistent spacing between functional parts of your code.
 
I have been rejected from a company that i applied for a job and sent an example application for them. The feedback was like that.

1- What means codebase here ? The example has a View controller with 3 buttons only.
2- Is there a source that i can learn unit test in a fast way ?
3- Do you put comments often while you are coding ? I generally use pragma mark
4- What means bad styling ?
5- What means no physical layouting ?
Here is the simple application i developed for them.
https://github.com/erdinckolukisa/GoEuro

Can you criticise my code please ?

I'll start with your post. Your headline say "some information". This is about as bad as saying "help me".
Look at it from someone else's view... someone sees a headline that says "some information" and it tells them nothing about the issue.

The better headline might be "my project got rejected because of code style".

Same thing with how a var or method is named. It saves time and makes it clear what the subject is.

Pragma marks are good for sorting things out, but meaningful comments make it easier for someone to jump in and work with your code.

Download some Git programs and look at the code, look at the comments and they should tell you exactly what is going on.

Remember, it's not only you that will be working with this program. A company is paying you to write code, they need something that can be maintained for years to come. It's not just about "make it work" ... it's about "make it so we can use it for years to come".

3rd party tools are used all the time. I use several of them. A menu slide out, RSS feed tool, etc... Look up a few and put some into your program.

I worked for years in a shared programming job, we had to use version control and I used to check things out, mess them up for testing and then clean it up before it went back into the system where others can work with them.

Think about how others are going to be able to read and understand your code. The company cares about how much time it take for others to dig thru your code and find the area that needs to be changed.

It's not just about making a program that works, it's about code a company pays you to write and will have to maintain after you're gone. Make the names clear.

The example of your title on this thread is a great example of what not to do, the title tells us nothing about the subject you are talking about.
 
You're right I didn't look in the folder for the storyboards, missed it.

Some pointer for your "selectTheType" Action, try and avoid iterating through your view hierarchy when searching for an item. Avoid using tags to find objects. Not in all cases, but there is usually a better way to solve a problem.

The biggest most blatant problem is how messy and disorganized your classes are. Don't keep your commented out code in the class and maintain consistent spacing between functional parts of your code.

I will be more careful about naming.

Is it better way to create an IBOutlet for each button and change button's some properties according to its current title ?
For example
Code:
if([sender.currentTitle isEqualToString:@"Bus"]) {
   [busButton setBackgroundImage:[UIImage imageNamed:@"buttonActiveImage.png"] forState:UIControlStateNormal];
   [trainButton setBackgroundImage:[UIImage imageNamed:@"buttonPassivaImage.png"] forState:UIControlStateNormal];
   [planeButton setBackgroundImage:[UIImage imageNamed:@"buttonPassiveImage.png"] forState:UIControlStateNormal];
}
else if([sender.currentTitle isEqualToString:@"Train"]){
[busButton setBackgroundImage:[UIImage imageNamed:@"buttonPassiveImage.png"] forState:UIControlStateNormal];
   [trainButton setBackgroundImage:[UIImage imageNamed:@"buttonActiveImage.png"] forState:UIControlStateNormal];
   [planeButton setBackgroundImage:[UIImage imageNamed:@"buttonPassiveImage.png"] forState:UIControlStateNormal];
}
else {
[busButton setBackgroundImage:[UIImage imageNamed:@"buttonPassiveImage.png"] forState:UIControlStateNormal];
   [trainButton setBackgroundImage:[UIImage imageNamed:@"buttonPassiveImage.png"] forState:UIControlStateNormal];
   [planeButton setBackgroundImage:[UIImage imageNamed:@"buttonActiveImage.png"] forState:UIControlStateNormal];
}
[doublepost=1474964445][/doublepost]
I'll start with your post. Your headline say "some information". This is about as bad as saying "help me".
Look at it from someone else's view... someone sees a headline that says "some information" and it tells them nothing about the issue.

The better headline might be "my project got rejected because of code style".

Same thing with how a var or method is named. It saves time and makes it clear what the subject is.

Pragma marks are good for sorting things out, but meaningful comments make it easier for someone to jump in and work with your code.

Download some Git programs and look at the code, look at the comments and they should tell you exactly what is going on.

Remember, it's not only you that will be working with this program. A company is paying you to write code, they need something that can be maintained for years to come. It's not just about "make it work" ... it's about "make it so we can use it for years to come".

3rd party tools are used all the time. I use several of them. A menu slide out, RSS feed tool, etc... Look up a few and put some into your program.

I worked for years in a shared programming job, we had to use version control and I used to check things out, mess them up for testing and then clean it up before it went back into the system where others can work with them.

Think about how others are going to be able to read and understand your code. The company cares about how much time it take for others to dig thru your code and find the area that needs to be changed.

It's not just about making a program that works, it's about code a company pays you to write and will have to maintain after you're gone. Make the names clear.

The example of your title on this thread is a great example of what not to do, the title tells us nothing about the subject you are talking about.

I use 3rd party tools also. But in that project i didnt need it actually. Maybe i could use them to cache images etc.

I will keep your suggestion about downloading github projects. It will help to improve my coding style maybe.

Thank you for your suggestions.
 
  • Like
Reactions: 1458279
Register on MacRumors! This sidebar will go away, and you'll see fewer ads.